fix(openscience): atomic + locked session writes, distinguish logout from read error (#131)

- saveSession uses atomic temp+rename so a crash or concurrent reader never
  sees a torn session file.
- updateSession serializes its read-modify-write under a lock, so two
  concurrent patches (interactive last_check_ts vs background cached_v) can't
  lose each other's field.
- getSession distinguishes a MISSING file (genuine logout → null) from a
  read/parse error (torn file, EMFILE, permissions): the latter is no longer
  silently mis-read as 'signed out' (which flipped the billing gate to BYOK and
  diverted usage to the unauthenticated queue for an authenticated user) — it
  still returns null so callers stay simple, but logs the failure. (#12)

Author: aayambansal

backend/cli/src/openscience/index.ts

@@ -336,6 +336,11 @@ export namespace OpenScience {
   }
 
   export async function getSession(): Promise<OpenScienceSession | null> {
+    // A missing file is a genuine logout → null, silently. Distinguish it from a
+    // read/parse error below so a torn file / EMFILE / permission blip isn't
+    // silently mis-read as "signed out" (which flips the billing gate to BYOK
+    // and diverts usage to the unauthenticated queue for an authed user).
+    if (!existsSync(filepath)) return null
     try {
       const file = Bun.file(filepath)
       const data = (await file.json()) as Partial<OpenScienceSession> & { access_token?: string }
@@ -359,13 +364,21 @@ export namespace OpenScience {
         cached_v: data.cached_v,
         last_check_ts: data.last_check_ts,
       }
-    } catch {
+    } catch (e) {
+      // The file exists but couldn't be read/parsed — NOT a logout. Return null
+      // so callers stay simple, but surface it so the transient failure is
+      // diagnosable rather than a silent, mis-billed "signed out".
+      log.warn("could not read session file (treating as signed out)", {
+        error: e instanceof Error ? e.message : String(e),
+      })
       return null
     }
   }
 
   export async function saveSession(session: OpenScienceSession) {
-    await Bun.write(Bun.file(filepath), JSON.stringify(session, null, 2), { mode: 0o600 })
+    // Atomic temp+rename so a crash or a concurrent reader never sees a torn
+    // session file (which getSession would mis-read as a logout).
+    await atomicWrite(filepath, JSON.stringify(session, null, 2), { mode: 0o600 })
     await ensureAtlasCliConfig(session)
   }
 
@@ -402,8 +415,12 @@ export namespace OpenScience {
   }
 
   /** Merge-update the persisted session. Fetches current session, spreads
-   *  the patch on top, and writes back. No-ops when unauthenticated. */
+   *  the patch on top, and writes back. No-ops when unauthenticated. Serialized
+   *  under a lock so two concurrent patches (e.g. an interactive last_check_ts
+   *  update racing a background cached_v update) can't lose each other's field
+   *  in the read-modify-write. */
   async function updateSession(patch: Partial<OpenScienceSession>): Promise<void> {
+    using _ = await Lock.write(filepath)
     const session = await getSession()
     if (!session) return
     await saveSession({ ...session, ...patch })

backend/cli/test/openscience/session-file.test.ts

@@ -0,0 +1,28 @@
+import { describe, expect, test } from "bun:test"
+import { OpenScience } from "../../src/openscience"
+
+// Test env is XDG-isolated (see test/preload.ts), so the session file lives in a
+// throwaway dir and starts absent.
+
+describe("OpenScience session file", () => {
+  test("getSession returns null when no session file exists (real logout)", async () => {
+    await OpenScience.clearSession()
+    expect(await OpenScience.getSession()).toBeNull()
+    expect(await OpenScience.isAuthenticated()).toBe(false)
+  })
+
+  test("saveSession then getSession round-trips atomically", async () => {
+    await OpenScience.saveSession({ api_key: "thk_test.secret", user_id: "u1", device_name: "dev" })
+    const s = await OpenScience.getSession()
+    expect(s?.api_key).toBe("thk_test.secret")
+    expect(s?.user_id).toBe("u1")
+    await OpenScience.clearSession()
+    expect(await OpenScience.getSession()).toBeNull()
+  })
+
+  test("a session without an api_key is treated as no session", async () => {
+    await OpenScience.saveSession({ api_key: "", user_id: "u1" } as any)
+    expect(await OpenScience.getSession()).toBeNull()
+    await OpenScience.clearSession()
+  })
+})