Skip to content

Commit bde6971

Browse files
adambaloghclaude
andauthored
fix(session): reload session.json so a fresh login reaches the running server (#14)
The background server loads the Chat session once at startup and keeps it in memory. After it is signed out upstream, requests fail with a 401 — and crucially they keep failing even after a successful `og-veil login`, because login writes a new token to disk while the running server keeps using its stale (expired/revoked) in-memory copy. The only recovery was an undiscoverable `og-veil restart`. Make Session re-read session.json when another process rewrites it (tracked by file mtime) before checking expiry/refreshing, so a fresh login is adopted by the running server on the next request without a restart. Guard refresh/reload with a lock since the server is threaded. Co-authored-by: Claude <noreply@anthropic.com>
1 parent f0a379f commit bde6971

2 files changed

Lines changed: 141 additions & 5 deletions

File tree

‎tests/test_session.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
"""Session persistence + auto-reload.
2+
3+
The long-running background server loads the session once at startup. These tests
4+
pin down the behavior that lets a fresh ``og-veil login`` reach that running
5+
server: the in-memory session re-reads ``session.json`` when another process
6+
rewrites it, instead of forever using its stale (expired/revoked) token.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import json
12+
import time
13+
14+
import pytest
15+
16+
from veil.config import session_path
17+
from veil.session import AuthError, Session
18+
19+
20+
@pytest.fixture
21+
def home(tmp_path, monkeypatch):
22+
monkeypatch.setenv("OG_VEIL_HOME", str(tmp_path))
23+
return tmp_path
24+
25+
26+
def _bundle(access_token: str, *, expires_at: float) -> dict:
27+
return {
28+
"type": "opengradient-cli-auth",
29+
"access_token": access_token,
30+
"refresh_token": "refresh-xyz",
31+
"expires_at": expires_at,
32+
"user": {"email": "me@example.com"},
33+
"config": {
34+
"supabase_url": "https://supabase.example",
35+
"supabase_anon_key": "anon",
36+
"chat_api_base_url": "https://chat.example",
37+
},
38+
}
39+
40+
41+
def _write_session(data: dict) -> None:
42+
session_path().write_text(json.dumps(data))
43+
44+
45+
def test_fresh_login_on_disk_is_picked_up_without_restart(home):
46+
# The server loaded a session whose token is already expired and whose refresh
47+
# token would be rejected upstream — the exact state behind the reported 401.
48+
_write_session(_bundle("stale-token", expires_at=time.time() - 3600))
49+
session = Session.load()
50+
51+
# The user runs `og-veil login`, which writes a brand-new, valid session to
52+
# disk from another process. The running server must adopt it.
53+
_write_session(_bundle("fresh-token", expires_at=time.time() + 3600))
54+
55+
# No refresh network call should be needed: the reloaded token is still valid.
56+
assert session.access_token() == "fresh-token"
57+
assert session.user_email == "me@example.com"
58+
59+
60+
def test_unchanged_session_is_not_reloaded_and_no_refresh(home):
61+
_write_session(_bundle("good-token", expires_at=time.time() + 3600))
62+
session = Session.load()
63+
64+
# Make any refresh attempt explode so the test fails loudly if one happens.
65+
def _boom():
66+
raise AssertionError("should not refresh a still-valid token")
67+
68+
session._refresh = _boom # type: ignore[assignment]
69+
assert session.access_token() == "good-token"
70+
71+
72+
def test_refresh_writes_back_without_triggering_self_reload(home):
73+
# A token that's expired in memory but has no fresh login on disk must still go
74+
# through refresh; the write-back must not look like an external change.
75+
_write_session(_bundle("expired-token", expires_at=time.time() - 3600))
76+
session = Session.load()
77+
78+
calls = {"n": 0}
79+
80+
def _fake_refresh():
81+
calls["n"] += 1
82+
session._data["access_token"] = "refreshed-token"
83+
session._data["expires_at"] = time.time() + 3600
84+
session.save()
85+
86+
session._refresh = _fake_refresh # type: ignore[assignment]
87+
assert session.access_token() == "refreshed-token"
88+
# A second call sees its own write (unchanged mtime), so it neither reloads nor
89+
# refreshes again.
90+
assert session.access_token() == "refreshed-token"
91+
assert calls["n"] == 1
92+
93+
94+
def test_load_without_session_file_raises(home):
95+
with pytest.raises(AuthError):
96+
Session.load()

‎veil/session.py‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from __future__ import annotations
1717

1818
import json
19+
import os
1920
import threading
2021
import time
2122
import webbrowser
@@ -66,9 +67,14 @@ def from_dict(cls, d: dict) -> "NetworkConfig":
6667
class Session:
6768
"""A persisted Chat session: tokens + network config, with auto-refresh."""
6869

69-
def __init__(self, data: dict):
70+
def __init__(self, data: dict, mtime: Optional[float] = None):
7071
self._data = data
7172
self.config = NetworkConfig.from_dict(data.get("config", {}))
73+
# Serializes refresh/reload across the threaded server's request handlers.
74+
self._lock = threading.Lock()
75+
# mtime of session.json as we last read/wrote it, so a long-running server
76+
# can notice when another process (e.g. `og-veil login`) rewrites it.
77+
self._mtime = mtime
7278

7379
# --- persistence -------------------------------------------------------
7480
@classmethod
@@ -77,7 +83,7 @@ def load(cls) -> "Session":
7783
if not path.exists():
7884
raise AuthError("not logged in — run `og-veil login` first")
7985
try:
80-
return cls(json.loads(path.read_text()))
86+
return cls(json.loads(path.read_text()), mtime=_file_mtime(path))
8187
except (OSError, json.JSONDecodeError) as exc:
8288
raise AuthError(f"could not read saved session: {exc}") from exc
8389

@@ -88,6 +94,28 @@ def save(self) -> None:
8894
path.chmod(0o600) # the file holds a live session token
8995
except OSError:
9096
pass
97+
# Record our own write so reload-on-change doesn't re-read it needlessly.
98+
self._mtime = _file_mtime(path)
99+
100+
def _reload_if_changed(self) -> None:
101+
"""Re-read session.json if another process rewrote it (e.g. a fresh login).
102+
103+
The background server loads the session once at startup and keeps it in
104+
memory. Without this, a successful ``og-veil login`` — which writes a new
105+
token to disk — would never reach the running server, so requests would
106+
keep failing with the stale (expired/revoked) token until a restart.
107+
"""
108+
path = session_path()
109+
disk_mtime = _file_mtime(path)
110+
if disk_mtime is None or disk_mtime == self._mtime:
111+
return
112+
try:
113+
data = json.loads(path.read_text())
114+
except (OSError, json.JSONDecodeError):
115+
return # mid-write or unreadable; keep what we have and try again later
116+
self._data = data
117+
self.config = NetworkConfig.from_dict(data.get("config", {}))
118+
self._mtime = disk_mtime
91119

92120
# --- accessors ---------------------------------------------------------
93121
@property
@@ -99,9 +127,13 @@ def auth_headers(self) -> dict:
99127
return {"Authorization": f"Bearer {self.access_token()}"}
100128

101129
def access_token(self) -> str:
102-
if self._is_expired():
103-
self._refresh()
104-
token = self._data.get("access_token")
130+
with self._lock:
131+
# Pick up a fresh login written by another process before deciding
132+
# whether the in-memory token needs refreshing.
133+
self._reload_if_changed()
134+
if self._is_expired():
135+
self._refresh()
136+
token = self._data.get("access_token")
105137
if not token:
106138
raise AuthError("session has no access token — run `og-veil login` to sign in again")
107139
return token
@@ -257,3 +289,11 @@ def _require(d: dict, key: str) -> str:
257289
if not value:
258290
raise AuthError(f"CLI-auth config is missing '{key}'")
259291
return value
292+
293+
294+
def _file_mtime(path) -> Optional[float]:
295+
"""Last-modified time of ``path`` in nanoseconds, or None if it's missing."""
296+
try:
297+
return os.stat(path).st_mtime_ns
298+
except OSError:
299+
return None

0 commit comments

Comments
 (0)