ananyajoshi commited on
Commit
b5d28f1
·
verified ·
1 Parent(s): b048ded

Fix: propagate session-only API key to background worker threads (proposal/analysis generation)

Browse files
Files changed (2) hide show
  1. app/config.py +53 -8
  2. app/services/background_tasks.py +23 -3
app/config.py CHANGED
@@ -8,6 +8,7 @@ never printed, logged, or included in `repr()`.
8
  from __future__ import annotations
9
 
10
  import os
 
11
  from dataclasses import dataclass, field
12
  from pathlib import Path
13
 
@@ -260,6 +261,15 @@ def _model_supports_vision(model: str | None) -> bool:
260
 
261
  _SESSION_ENV_KEY = "_session_env"
262
 
 
 
 
 
 
 
 
 
 
263
 
264
  def _has_streamlit_ctx() -> bool:
265
  """True only inside an active Streamlit script run.
@@ -278,15 +288,50 @@ def _has_streamlit_ctx() -> bool:
278
 
279
 
280
  def _session_store() -> dict | None:
281
- """Return the live per-session override dict, or ``None`` when headless."""
282
- if not _has_streamlit_ctx():
283
- return None
284
- try:
285
- import streamlit as st
286
 
287
- return st.session_state.setdefault(_SESSION_ENV_KEY, {})
288
- except Exception: # pragma: no cover - defensive
289
- return None
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
290
 
291
 
292
  def _env(name: str, default: str | None = None) -> str | None:
 
8
  from __future__ import annotations
9
 
10
  import os
11
+ import threading
12
  from dataclasses import dataclass, field
13
  from pathlib import Path
14
 
 
261
 
262
  _SESSION_ENV_KEY = "_session_env"
263
 
264
+ # Background-worker fallback. A worker thread (see
265
+ # :mod:`app.services.background_tasks`) has NO Streamlit script-run context, so
266
+ # ``st.session_state`` — and therefore the session-only API key — is invisible
267
+ # to it. Before launching a worker, the main thread snapshots the session
268
+ # overrides and the worker installs them here, in its OWN thread-local. Because
269
+ # the store is per-thread, one visitor's key can never leak into another
270
+ # visitor's concurrent worker. Outside a worker this is simply unset.
271
+ _thread_local = threading.local()
272
+
273
 
274
  def _has_streamlit_ctx() -> bool:
275
  """True only inside an active Streamlit script run.
 
288
 
289
 
290
  def _session_store() -> dict | None:
291
+ """Return the active per-session override dict, or ``None`` when absent.
 
 
 
 
292
 
293
+ Resolution order:
294
+
295
+ 1. Inside a Streamlit script run → the live ``st.session_state`` store.
296
+ 2. Inside a background worker that had overrides installed → its
297
+ thread-local store (see :func:`install_thread_overrides`).
298
+ 3. Otherwise (tests / bare mode) → ``None``.
299
+ """
300
+ if _has_streamlit_ctx():
301
+ try:
302
+ import streamlit as st
303
+
304
+ return st.session_state.setdefault(_SESSION_ENV_KEY, {})
305
+ except Exception: # pragma: no cover - defensive
306
+ return None
307
+ return getattr(_thread_local, "store", None)
308
+
309
+
310
+ def export_session_overrides() -> dict:
311
+ """Snapshot the current session's overrides (call on the main thread).
312
+
313
+ Returns a plain copy — including any API key — so a background worker can
314
+ carry the launching session's config across the thread boundary. Never log
315
+ the result.
316
+ """
317
+ store = _session_store()
318
+ return dict(store) if store else {}
319
+
320
+
321
+ def install_thread_overrides(overrides: dict) -> None:
322
+ """Install ``overrides`` for the CURRENT thread (call inside the worker).
323
+
324
+ Stored in a thread-local so concurrent workers stay isolated. Also drops any
325
+ stale per-thread Settings cache so the next :func:`get_settings` rebuilds
326
+ from the freshly installed values.
327
+ """
328
+ store = dict(overrides or {})
329
+ _thread_local.store = store
330
+
331
+
332
+ def clear_thread_overrides() -> None:
333
+ """Remove this thread's installed overrides (call in the worker's finally)."""
334
+ _thread_local.store = None
335
 
336
 
337
  def _env(name: str, default: str | None = None) -> str | None:
app/services/background_tasks.py CHANGED
@@ -10,9 +10,16 @@ keeps going and the result is waiting when they return.
10
 
11
  IMPORTANT: the worker function must not touch ``st.session_state`` (Streamlit
12
  calls from a thread with no script-run context fail). Pass everything it needs
13
- as arguments and read the result back on the main thread. ``get_settings()`` is
14
- safe (it only reads a cached ``.env``); the LLM client's usage logging is
15
- already guarded against a missing session.
 
 
 
 
 
 
 
16
  """
17
 
18
  from __future__ import annotations
@@ -20,6 +27,8 @@ from __future__ import annotations
20
  import threading
21
  from typing import Any, Callable
22
 
 
 
23
  _lock = threading.Lock()
24
  _tasks: dict[str, dict[str, Any]] = {}
25
 
@@ -50,7 +59,15 @@ def start(task_id: str, fn: Callable[..., Any], *args: Any, **kwargs: Any) -> bo
50
  return False
51
  _tasks[task_id] = {"status": "running", "result": None, "error": None}
52
 
 
 
 
 
 
 
53
  def _run() -> None:
 
 
54
  try:
55
  res = fn(*args, **kwargs)
56
  with _lock:
@@ -58,6 +75,9 @@ def start(task_id: str, fn: Callable[..., Any], *args: Any, **kwargs: Any) -> bo
58
  except Exception as exc: # noqa: BLE001 - surface as a clean error status
59
  with _lock:
60
  _tasks[task_id] = {"status": "error", "result": None, "error": str(exc)}
 
 
 
61
 
62
  if _has_streamlit_runtime():
63
  threading.Thread(target=_run, daemon=True).start()
 
10
 
11
  IMPORTANT: the worker function must not touch ``st.session_state`` (Streamlit
12
  calls from a thread with no script-run context fail). Pass everything it needs
13
+ as arguments and read the result back on the main thread.
14
+
15
+ API-key propagation: the API key now lives only in the per-session store
16
+ (``st.session_state``), which a worker thread cannot see. So before spawning the
17
+ worker we snapshot the launching session's config overrides on the MAIN thread
18
+ and re-install them into the worker's own thread-local (see
19
+ ``app.config.export_session_overrides`` / ``install_thread_overrides``). This
20
+ makes ``get_settings()`` inside the worker resolve the same key/provider/model
21
+ the user configured — without leaking it to any other visitor's worker. The LLM
22
+ client's usage logging is already guarded against a missing session.
23
  """
24
 
25
  from __future__ import annotations
 
27
  import threading
28
  from typing import Any, Callable
29
 
30
+ from app import config
31
+
32
  _lock = threading.Lock()
33
  _tasks: dict[str, dict[str, Any]] = {}
34
 
 
59
  return False
60
  _tasks[task_id] = {"status": "running", "result": None, "error": None}
61
 
62
+ # Snapshot the launching session's config (incl. the session-only API key)
63
+ # HERE, on the main thread, while st.session_state is reachable. The worker
64
+ # re-installs it into its own thread-local so get_settings() resolves the
65
+ # right key. Empty when no session (tests / bare mode).
66
+ session_overrides = config.export_session_overrides()
67
+
68
  def _run() -> None:
69
+ if session_overrides:
70
+ config.install_thread_overrides(session_overrides)
71
  try:
72
  res = fn(*args, **kwargs)
73
  with _lock:
 
75
  except Exception as exc: # noqa: BLE001 - surface as a clean error status
76
  with _lock:
77
  _tasks[task_id] = {"status": "error", "result": None, "error": str(exc)}
78
+ finally:
79
+ if session_overrides:
80
+ config.clear_thread_overrides()
81
 
82
  if _has_streamlit_runtime():
83
  threading.Thread(target=_run, daemon=True).start()