fix: auth/sync robustness (5 review mediums)

src/auth/auth-flow.test.ts

@@ -102,4 +102,26 @@ describe('auth-flow', () => {
     expect(post).not.toHaveBeenCalled();
     expect(await flow(fakeStore(), post).f.getValidToken()).toBeNull();
   });
+
+  it('getValidToken clears the dead session (and notifies onChange) on a 4xx refresh', async () => {
+    const store = fakeStore({ accessToken: 'AT0', refreshToken: 'RT0', expiresAt: NOW - 1 }, 'cid');
+    const post = vi.fn(async () => ({ status: 400, json: { error: 'invalid_grant' } }));
+    const onChange = vi.fn();
+    const notify = vi.fn();
+    const tok = await flow(store, post, { onChange, notify }).f.getValidToken();
+    expect(tok).toBeNull();
+    expect(store.load()).toBeNull(); // session cleared → UI flips to signed-out
+    expect(onChange).toHaveBeenCalled();
+    expect(notify).toHaveBeenCalledWith(expect.stringContaining('session expired'));
+  });
+
+  it('getValidToken keeps tokens on a transient 5xx refresh (retryable, not a sign-out)', async () => {
+    const store = fakeStore({ accessToken: 'AT0', refreshToken: 'RT0', expiresAt: NOW - 1 }, 'cid');
+    const post = vi.fn(async () => ({ status: 503, json: null }));
+    const onChange = vi.fn();
+    const tok = await flow(store, post, { onChange }).f.getValidToken();
+    expect(tok).toBeNull();
+    expect(store.load()).not.toBeNull(); // tokens preserved
+    expect(onChange).not.toHaveBeenCalled();
+  });
 });

src/auth/auth-flow.ts

@@ -11,6 +11,7 @@ import {
 import {
   exchangeAuthCode,
   isTokenExpired,
+  OAuthHttpError,
   refreshTokens,
   registerClient,
   revokeToken,
@@ -143,7 +144,15 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
       );
       await deps.store.save(refreshed);
       return refreshed.accessToken;
-    } catch {
+    } catch (e) {
+      // A rejected refresh (4xx: invalid/expired/revoked refresh token) means the session is
+      // dead — clear it so the UI flips to signed-out instead of a green dot that fails every
+      // sync. Keep tokens on transient/server errors (5xx/network) so a blip isn't a sign-out.
+      if (e instanceof OAuthHttpError && e.status >= 400 && e.status < 500) {
+        await deps.store.clear();
+        deps.onChange?.();
+        deps.notify('Your Agentage session expired — sign in again.');
+      }
       return null;
     }
   };

src/auth/oauth.ts

@@ -19,6 +19,18 @@ export interface HttpResponse {
   status: number;
   json: unknown;
 }
+
+// Carries the HTTP status so callers can tell a rejected grant (4xx — clear the session)
+// from a transient/server error (5xx/network — keep tokens, retry later).
+export class OAuthHttpError extends Error {
+  constructor(
+    readonly status: number,
+    message: string
+  ) {
+    super(message);
+    this.name = 'OAuthHttpError';
+  }
+}
 export type HttpPost = (
   url: string,
   init: { headers: Record<string, string>; body: string }
@@ -70,7 +82,8 @@ async function postToken(
   const res = await post(tokenEndpoint, { headers: FORM, body: form(body) });
   if (res.status < 200 || res.status >= 300) {
     const d = res.json as { error_description?: string; error?: string } | null;
-    throw new Error(
+    throw new OAuthHttpError(
+      res.status,
       `Token request failed: ${d?.error_description ?? d?.error ?? `HTTP ${res.status}`}`
     );
   }

src/git/git-client.ts

@@ -25,6 +25,9 @@ export interface RepoCtx {
 
 export interface PullResult {
   conflicted: string[];
+  // True when the histories can't be 3-way merged automatically (criss-cross / multiple merge
+  // bases — MergeNotSupportedError). No filepaths; the caller surfaces an actionable note.
+  unmergeable?: boolean;
 }
 
 const AUTHOR = { name: 'Agentage Memory', email: 'memory@agentage.io' };
@@ -103,23 +106,35 @@ export function createGitClient({ fs, http }: GitClientDeps, mergeDriver: MergeD
           // (abortOnConflict:false) — do NOT checkout, that would discard them.
           return { conflicted: e.data.filepaths };
         }
-        throw e; // MergeNotSupportedError (criss-cross) handled by the caller (backup + LWW)
+        if (e instanceof Errors.MergeNotSupportedError) {
+          // Criss-cross / multiple merge bases: iso-git can't auto-merge and the worktree is
+          // left intact. Signal the caller to surface a clear "resolve manually" note rather
+          // than bubbling a raw error. (No auto LWW fallback — it would risk silent data loss.)
+          return { conflicted: [], unmergeable: true };
+        }
+        throw e;
       }
     },
 
     // No force. A non-ff push throws PushRejectedError → re-pull + re-push (still no force).
-    async push(c: RepoCtx): Promise<void> {
+    // Returns the conflict outcome: an empty result = pushed; a conflicted/unmergeable result
+    // means the re-pull hit a conflict and we did NOT push — the caller surfaces it.
+    async push(c: RepoCtx): Promise<PullResult> {
       const ref = c.ref ?? 'main';
       const doPush = () => wrapFS(git.push({ ...base(c), ...auth(c), http, url: c.url, ref }));
       try {
         const r = await doPush();
         if (!r.ok) throw new Error('push not ok');
+        return { conflicted: [] };
       } catch (e) {
         if (e instanceof Errors.PushRejectedError) {
-          await client.pull(c);
+          // Someone pushed between our pull and push. Re-pull (merge); if THAT conflicts,
+          // propagate it instead of blindly re-pushing into a "not ok after rebase" error.
+          const pulled = await client.pull(c);
+          if (pulled.conflicted.length > 0 || pulled.unmergeable) return pulled;
           const r2 = await doPush();
           if (!r2.ok) throw new Error('push not ok after rebase');
-          return;
+          return { conflicted: [] };
         }
         throw e;
       }

src/main.ts

@@ -3,6 +3,7 @@ import type { FsClient, MergeDriverCallback } from 'isomorphic-git';
 import {
   type AgentageMemorySettings,
   type VaultInfo,
+  type VaultListResult,
   DEFAULT_SETTINGS,
   normalizeVaultName,
 } from './settings';
@@ -355,24 +356,29 @@ export default class AgentageMemoryPlugin extends Plugin implements SettingsHost
   }
 
   // --- memory selection (SettingsHost) ---
-  /** Existing server memories + their info (files/folders/updated) via GET /vaults. */
-  async listVaults(): Promise<VaultInfo[]> {
+  /** Existing server memories + their info (files/folders/updated) via GET /vaults. Returns a
+   * result so the chooser can show a real error (with Retry) vs an empty account. */
+  async listVaults(): Promise<VaultListResult> {
     const token = await this.auth.getValidToken();
-    if (!token) return [];
+    if (!token) return { ok: false, error: 'Sign in to Agentage first.' };
     try {
       const r = await requestUrl({
         url: `${SYNC_ORIGIN}/vaults`,
         method: 'GET',
         headers: { Authorization: `Bearer ${token}` },
         throw: false,
       });
-      if (r.status < 200 || r.status >= 300) return [];
+      if (r.status < 200 || r.status >= 300) {
+        const body = safeJson(r.text) as { error?: string } | null;
+        return { ok: false, error: body?.error ?? `Couldn't load memories (HTTP ${r.status})` };
+      }
       const raw = (safeJson(r.text) as { vaults?: unknown })?.vaults;
-      return Array.isArray(raw)
+      const vaults = Array.isArray(raw)
         ? raw.map(parseVaultInfo).filter((v): v is VaultInfo => v !== null)
         : [];
-    } catch {
-      return [];
+      return { ok: true, vaults };
+    } catch (e) {
+      return { ok: false, error: (e as Error).message };
     }
   }
 
@@ -441,8 +447,14 @@ export default class AgentageMemoryPlugin extends Plugin implements SettingsHost
   isSignedIn(): boolean {
     return this.auth.isSignedIn();
   }
-  disconnect(): Promise<void> {
-    return this.auth.disconnect();
+  async disconnect(): Promise<void> {
+    await this.auth.disconnect();
+    // Drop the selected memory so the next session starts in the amber "Choose Memory" state
+    // and re-validates the pick — never resurface a stale (possibly deleted) memory as active.
+    this.settings.vault = '';
+    this.lastVault = undefined;
+    await this.saveSettings();
+    this.refreshStatus();
   }
 
   // Git fs runs over Obsidian's vault adapter on BOTH desktop and mobile (the proven
@@ -503,6 +515,8 @@ export default class AgentageMemoryPlugin extends Plugin implements SettingsHost
           ok: false,
           message: `Conflicts in ${r.conflicted.length} file(s) — see "Agentage Sync Conflicts".`,
         };
+      // Unmergeable history (criss-cross): not pushed, no per-file markers — surface the reason.
+      if (!r.pushed && r.message) return { ok: false, message: r.message };
       const bits = [r.action, r.committed ? 'committed' : '', r.pushed ? 'pushed' : ''].filter(
         Boolean
       );

src/memory-chooser.ts

@@ -1,9 +1,9 @@
 import { type App, Modal, Notice, Setting } from 'obsidian';
-import type { VaultInfo } from './settings';
+import type { VaultInfo, VaultListResult } from './settings';
 
 // What the chooser needs from the plugin. Kept narrow so it doesn't depend on main.
 export interface MemoryChooserHost {
-  listVaults(): Promise<VaultInfo[]>;
+  listVaults(): Promise<VaultListResult>;
   createVault(name: string): Promise<{ ok: boolean; vault?: string; error?: string }>;
   defaultVaultName(): string;
   selectVault(name: string): Promise<void>;
@@ -78,29 +78,34 @@ class MemoryModal extends Modal {
   }
 
   private async fillList(): Promise<void> {
-    const [vaults, cur] = [await this.host.listVaults(), this.host.currentVault()];
+    const [res, cur] = [await this.host.listVaults(), this.host.currentVault()];
     this.listEl.empty();
-    if (!vaults.length) {
+    if (!res.ok) {
+      this.listEl.createEl('p', {
+        text: `Couldn't load your memories: ${res.error}`,
+        cls: 'ams-hint',
+      });
+      new Setting(this.listEl).addButton((b) =>
+        b.setButtonText('Retry').onClick(() => {
+          this.listEl.empty();
+          this.listEl.createEl('p', { text: 'Loading your memories…', cls: 'ams-hint' });
+          void this.fillList();
+        })
+      );
+      return;
+    }
+    if (!res.vaults.length) {
       this.listEl.createEl('p', { text: 'No memories yet — create one below.', cls: 'ams-hint' });
       return;
     }
-    for (const v of vaults) {
+    for (const v of res.vaults) {
       const isCur = v.name === cur;
       const meta = describeVault(v);
       const row = new Setting(this.listEl)
         .setName(v.name)
-        .setDesc(isCur ? `current · ${meta}` : meta)
-        .addButton((b) =>
-          b
-            .setButtonText(isCur ? 'In use' : 'Use')
-            .setDisabled(isCur)
-            .onClick(async () => {
-              await this.host.selectVault(v.name);
-              this.close();
-            })
-        );
-      // Sync now only for the memory in use; switch to another with Use first.
+        .setDesc(isCur ? `current · ${meta}` : meta);
       if (isCur) {
+        // Already in use → offer Sync now.
         row.addButton((b) =>
           b
             .setCta()
@@ -111,6 +116,25 @@ class MemoryModal extends Modal {
               this.close();
             })
         );
+      } else {
+        // Switch to it: Use (select only) or Use & sync (select + sync immediately).
+        row
+          .addButton((b) =>
+            b.setButtonText('Use').onClick(async () => {
+              await this.host.selectVault(v.name);
+              this.close();
+            })
+          )
+          .addButton((b) =>
+            b
+              .setCta()
+              .setButtonText('Use & sync')
+              .onClick(async () => {
+                b.setDisabled(true).setButtonText('Syncing…');
+                await this.host.syncVault(v.name);
+                this.close();
+              })
+          );
       }
     }
   }

src/settings.ts

@@ -47,6 +47,10 @@ export interface VaultInfo {
   updated: string | null;
   empty: boolean;
 }
+
+// Result of listing memories: lets the chooser tell a genuinely empty account ({ok,vaults:[]})
+// from a server/network failure ({ok:false}), instead of both rendering as "No memories yet".
+export type VaultListResult = { ok: true; vaults: VaultInfo[] } | { ok: false; error: string };
 export const DEFAULT_REMOTE_HOST = 'https://sync.agentage.io';
 export const VAULTS_SCHEMA_URL = 'https://memory.agentage.io/schema/vaults.json';
 // The managed remote alias. memory-core resolves "agentage" to the cloud git URL

src/sync-controller.ts

@@ -177,14 +177,41 @@ export function createSyncController(deps: SyncControllerDeps) {
     const staged = await stageChanges(ctx, new Set());
     if (staged > 0) await deps.client.commit(ctx, `Sync ${deps.now()}`);
 
-    const { conflicted } = await deps.client.pull(ctx);
-    if (conflicted.length > 0) {
-      await writeConflictNote(conflicted);
-      status('conflict', `${conflicted.length} file(s) need resolution`);
-      return { action: 'synced', committed: staged > 0, pushed: false, conflicted };
+    const pulled = await deps.client.pull(ctx);
+    if (pulled.unmergeable) {
+      const message =
+        "Automatic merge isn't possible for this history (diverged too far). " +
+        'Resolve in another client or re-clone, then sync again.';
+      status('conflict', message);
+      return { action: 'synced', committed: staged > 0, pushed: false, conflicted: [], message };
+    }
+    if (pulled.conflicted.length > 0) {
+      await writeConflictNote(pulled.conflicted);
+      status('conflict', `${pulled.conflicted.length} file(s) need resolution`);
+      return {
+        action: 'synced',
+        committed: staged > 0,
+        pushed: false,
+        conflicted: pulled.conflicted,
+      };
     }
 
-    await deps.client.push(ctx);
+    const pushed = await deps.client.push(ctx);
+    if (pushed.unmergeable || pushed.conflicted.length > 0) {
+      // A concurrent push landed; the re-pull conflicted — surface it instead of erroring.
+      if (pushed.conflicted.length > 0) await writeConflictNote(pushed.conflicted);
+      const message = pushed.unmergeable
+        ? "Automatic merge isn't possible for this history (diverged too far). Resolve and sync again."
+        : `${pushed.conflicted.length} file(s) need resolution`;
+      status('conflict', message);
+      return {
+        action: 'synced',
+        committed: staged > 0,
+        pushed: false,
+        conflicted: pushed.conflicted,
+        message,
+      };
+    }
     status('idle');
     return { action: 'synced', committed: staged > 0, pushed: true, conflicted: [] };
   }