diff --git a/backend/apps/help/changelog.py b/backend/apps/help/changelog.py index 1b023985..9934af29 100644 --- a/backend/apps/help/changelog.py +++ b/backend/apps/help/changelog.py @@ -86,6 +86,7 @@ P_RELEASES: List[ReleaseNote] = [ "Clicking empty canvas clears a text selection, the way every other text surface does.", "Brand-new apps stop dying at birth on busy machines. The first boot installs dependencies, which can take minutes; a fixed 60-second limit was killing exactly those boots.", "An agent that sent work to a browser can no longer hang forever when the finished result gets lost on the way back; it notices, recovers, and redoes the step.", + "The app opens seconds faster on machines with a crowded system temp folder. File uploads moved into OpenSwarm's own folder, so startup no longer pays a toll that grew with years of temp-file clutter.", ], ), ReleaseNote( diff --git a/backend/apps/settings/settings.py b/backend/apps/settings/settings.py index 1c227922..9e05a376 100644 --- a/backend/apps/settings/settings.py +++ b/backend/apps/settings/settings.py @@ -76,7 +76,7 @@ async def settings_lifespan(): async def p_upload_dir_gc_loop(): - """Daily GC of UPLOAD_DIR. Without this, every PDF/image the user + """Daily GC of the upload dir (and the legacy temp-dir location). Without this, every PDF/image the user drops sits in the OS temp dir forever, growing unbounded across sessions. We keep files for 7 days to make resume-after-restart work, then delete. macOS temp under /var/folders/... is auto-purged @@ -88,9 +88,11 @@ async def p_upload_dir_gc_loop(): try: now = time.time() cutoff = now - 7 * 86400 - if os.path.isdir(UPLOAD_DIR): - for entry in os.listdir(UPLOAD_DIR): - p = os.path.join(UPLOAD_DIR, entry) + for p_dir in (upload_dir(), p_legacy_upload_dir()): + if not os.path.isdir(p_dir): + continue + for entry in os.listdir(p_dir): + p = os.path.join(p_dir, entry) try: if os.path.isfile(p) and os.path.getmtime(p) < cutoff: os.remove(p) @@ -406,8 +408,20 @@ class BrowseResponse(BaseModel): files: list[str] -UPLOAD_DIR = os.path.join(tempfile.gettempdir(), "self-swarm-uploads") -os.makedirs(UPLOAD_DIR, exist_ok=True) +# Uploads live under OUR data dir, not the OS temp dir. Two reasons, both measured on the machine +# that reported the 22s LCP: tempfile.gettempdir() PROBES the temp dir (create+unlink) and the +# makedirs stats it, which cost 2.9s + 3.1s AT IMPORT on a temp dir grown to unlistable size (the +# same monster that killed dictation, ENG-312); and our own dir is GC'd by us, not the OS's whims. +def upload_dir() -> str: + from backend.config.paths import DATA_ROOT + d = os.path.join(DATA_ROOT, "uploads") + os.makedirs(d, exist_ok=True) + return d + + +# Old location, swept by the GC loop only (never probed at import; one stat when the loop runs). +def p_legacy_upload_dir() -> str: + return os.path.join(tempfile.gettempdir(), "self-swarm-uploads") def sniff_file_kind(contents: bytes, name: str) -> tuple[str, str | None]: @@ -506,7 +520,7 @@ async def upload_files(files: list[UploadFile] = File(...)): # Atomic create-with-collision-retry so two concurrent uploads with the same filename never overwrite each other. The previous exists() then open() pattern had a race window: both callers would observe `dest` free and both would write, with the second winning. O_EXCL fails the create if anyone else got there first. base, ext = os.path.splitext(safe_name) - dest = os.path.join(UPLOAD_DIR, safe_name) + dest = os.path.join(upload_dir(), safe_name) counter = 0 fd = None while fd is None: @@ -516,7 +530,7 @@ async def upload_files(files: list[UploadFile] = File(...)): counter += 1 if counter > 10_000: raise HTTPException(status_code=500, detail="upload dedup exhausted") - dest = os.path.join(UPLOAD_DIR, f"{base}_{counter}{ext}") + dest = os.path.join(upload_dir(), f"{base}_{counter}{ext}") try: with os.fdopen(fd, "wb") as fh: fh.write(contents) @@ -568,7 +582,7 @@ async def summarize_file(req: p_SummarizeRequest): Called from the chat-input attach handler when one file alone would exceed 50% of the selected model's context window. The summary is - written to a sibling file with `.summary.txt` suffix in UPLOAD_DIR so + written to a sibling file with `.summary.txt` suffix in the upload dir so the existing attachment plumbing (paths flow through context_paths) works unchanged. Aux model picked via provider-agnostic resolve_aux_model, so users on OpenAI/Gemini/OpenRouter get summarized @@ -577,7 +591,8 @@ async def summarize_file(req: p_SummarizeRequest): src = req.path if not os.path.isfile(src): raise HTTPException(status_code=404, detail="file not found") - if not os.path.commonpath([os.path.realpath(src), os.path.realpath(UPLOAD_DIR)]) == os.path.realpath(UPLOAD_DIR): + p_updir = upload_dir() + if not os.path.commonpath([os.path.realpath(src), os.path.realpath(p_updir)]) == os.path.realpath(p_updir): raise HTTPException(status_code=400, detail="path outside upload dir") try: diff --git a/backend/tests/test_upload_dir_off_tempdir.py b/backend/tests/test_upload_dir_off_tempdir.py new file mode 100644 index 00000000..386d4b6a --- /dev/null +++ b/backend/tests/test_upload_dir_off_tempdir.py @@ -0,0 +1,43 @@ +"""Importing settings must never pay the OS temp-dir toll (ENG-312, measured on Eric's machine). + +cProfile of `import backend.apps.settings.settings` on the packaged python attributed 6.0 of 6.6 +seconds to two module-level lines: `tempfile.gettempdir()` (2.9s, it PROBES the temp dir with a +create+unlink) and `os.makedirs(UPLOAD_DIR)` (3.1s of stats), because that temp dir has grown to +unlistable size (a bare listdir times out at 10s; the same monster that killed dictation). Uploads +now live under DATA_ROOT, created lazily on first use, and the GC loop sweeps the legacy location. +""" +import ast +import os + +from backend.apps.settings.settings import p_legacy_upload_dir, upload_dir + + +def test_no_module_level_tempdir_or_makedirs(): + import backend.apps.settings.settings as mod + + src = open(mod.__file__).read() + tree = ast.parse(src) + offenders = [] + for node in tree.body: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + continue + for sub in ast.walk(node): + if isinstance(sub, ast.Call): + target = ast.unparse(sub.func) + if "gettempdir" in target or "makedirs" in target: + offenders.append((node.lineno, target)) + assert not offenders, f"import-time temp-dir toll is back: {offenders}" + + +def test_upload_dir_lives_under_data_root_and_creates_itself(tmp_path, monkeypatch): + import backend.apps.settings.settings as mod + + monkeypatch.setattr("backend.config.paths.DATA_ROOT", str(tmp_path)) + d = mod.upload_dir() + assert d.startswith(str(tmp_path)), "uploads must live in OUR dir, never the OS temp dir" + assert os.path.isdir(d), "first use creates it" + + +def test_legacy_location_is_still_named_for_the_gc_sweep(): + # The old temp-dir location keeps getting swept so past uploads do not sit there for 7 years. + assert p_legacy_upload_dir().endswith("self-swarm-uploads")