diff --git a/CHANGELOG.md b/CHANGELOG.md index 5244c587..a909f3eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,16 @@ ## [Unreleased] +## [v0.51.427] — 2026-06-15 — Release ON (CLI sessions on by default for new installs #3988 + symlink delete/rename guard #4217) + +### Changed + +- **CLI / TUI / messaging sessions now appear in the sidebar by default for new installs (#3988).** `show_cli_sessions` now defaults to `True`, so sessions from the CLI, TUI, Telegram, Discord, and the Hermes One desktop app show up in the WebUI sidebar without users having to discover the toggle in Settings. **Existing installs are grandfathered:** a user who already completed onboarding and never opted in keeps their previous (hidden) behavior — the default change only affects fresh installs. The toggle remains in Settings → Sessions for anyone who wants to change it. + +### Fixed + +- **Deleting or renaming a workspace symlink no longer destroys the file or directory it points to (#4217).** `/api/file/delete` and `/api/file/rename` resolved the final symlink before operating on the target, so a standard delete/rename against a symlink name acted on the real (possibly out-of-workspace) target. Both handlers now reject a symlinked path with a 400, matching the guard the move handler already had. (#4217) + ## [v0.51.426] — 2026-06-15 — Release OM (custom-provider model-prefix routing fix, #4210) ### Fixed diff --git a/api/config.py b/api/config.py index c0073975..2e648ce3 100644 --- a/api/config.py +++ b/api/config.py @@ -6505,7 +6505,7 @@ _SETTINGS_DEFAULTS = { "hide_empty_state_suggestions": False, # hide the default new-chat suggestion buttons "show_tps": False, # show tokens-per-second chip in assistant message headers "fade_text_effect": False, # animate newly streamed words with a lightweight fade-in effect - "show_cli_sessions": False, # merge CLI sessions from state.db into the sidebar + "show_cli_sessions": True, # merge CLI/TUI/messaging sessions from state.db into the sidebar by default (#3988); established installs are grandfathered OFF by the load_settings backfill "show_cron_sessions": False, # surface cron sessions in the sidebar (subordinate to show_cli_sessions) "show_previous_messaging_sessions": False, # show older Telegram/Discord/etc. reset segments "sync_to_insights": False, # mirror WebUI token usage to state.db for /insights @@ -6647,6 +6647,26 @@ def load_settings() -> dict: if k not in _SETTINGS_LEGACY_DROP_KEYS } ) + # Grandfather established installs OFF for show_cli_sessions (#3988). + # The default flipped True so NEW users see CLI/TUI/messaging + # sessions without hunting for the toggle — but an existing user + # who never opted in should not have their sidebar silently change. + # Treat the install as established (and pin the old False default) + # when show_cli_sessions is absent AND the file already carries + # real user state — either onboarding was completed, or some + # setting OTHER than a not-yet-completed onboarding flag has been + # persisted. Keying on "has saved user state" (not just + # onboarding_completed) also covers a CLI-configured user who + # tweaked a WebUI setting before running the wizard. A genuinely + # new / still-mid-onboarding file falls through to the True default. + _established_keys = [ + k for k in stored + if k not in ("show_cli_sessions", "onboarding_completed") + ] + if "show_cli_sessions" not in stored and ( + bool(stored.get("onboarding_completed")) or _established_keys + ): + settings["show_cli_sessions"] = False except Exception: logger.debug("Failed to load settings from %s", SETTINGS_FILE) settings["theme"], settings["skin"] = _normalize_appearance( diff --git a/api/routes.py b/api/routes.py index 5e77372f..9daa71ff 100644 --- a/api/routes.py +++ b/api/routes.py @@ -14723,6 +14723,13 @@ def _handle_file_delete(handler, body): try: ws_root = Path(s.workspace) target = safe_resolve(ws_root, body["path"]) + # Reject a symlinked entry BEFORE the follow-based exists() check: a + # dangling symlink resolves to a missing target, so an exists()-first + # order would misclassify it as 404 "File not found" and leave it + # permanently undeletable. is_symlink() is a no-follow lstat on the + # lexically-requested path, so it catches both live and dangling links. + if (ws_root / body["path"]).is_symlink(): + return bad(handler, "Cannot delete a symlinked entry") if not target.exists(): return bad(handler, "File not found", 404) if target.is_dir(): @@ -14803,6 +14810,11 @@ def _handle_file_rename(handler, body): ws_root = Path(s.workspace) ws_root_resolved = ws_root.resolve() source = safe_resolve(ws_root, body["path"]) + # Reject a symlinked entry BEFORE the follow-based exists() check (see + # _handle_file_delete): a dangling symlink would otherwise 404 and stay + # unrenameable. is_symlink() is a no-follow lstat on the requested path. + if (ws_root / body["path"]).is_symlink(): + return bad(handler, "Cannot rename a symlinked entry") if not source.exists(): return bad(handler, "File not found", 404) new_name = body["new_name"].strip() @@ -14838,15 +14850,18 @@ def _handle_file_move(handler, body): # returning a confusing 400 for a move that actually happened. ws_root_resolved = ws_root.resolve() source = safe_resolve(ws_root, body["path"]) - if not source.exists(): - return bad(handler, "File not found", 404) - # Reject a symlinked SOURCE entry. safe_resolve() follows the final - # symlink, so source.name/source.parent would point at the link's - # TARGET, not the dragged entry — moving link.txt would silently move - # dir/real.txt and leave link.txt dangling. Detect the symlink on the - # lexically-requested final component (lstat, no-follow) and refuse. + # Reject a symlinked SOURCE entry BEFORE the follow-based exists() check. + # safe_resolve() follows the final symlink, so source.name/source.parent + # would point at the link's TARGET, not the dragged entry — moving + # link.txt would silently move dir/real.txt and leave link.txt dangling. + # Detect the symlink on the lexically-requested final component (lstat, + # no-follow) and refuse; running this before exists() also means a + # dangling symlink is rejected (400) rather than misclassified as 404 + # (matches the delete/rename ordering). if (ws_root / body["path"]).is_symlink(): return bad(handler, "Cannot move a symlinked entry") + if not source.exists(): + return bad(handler, "File not found", 404) dest_dir_raw = (body.get("dest_dir") or ".").strip() if not dest_dir_raw: dest_dir_raw = "." diff --git a/static/boot.js b/static/boot.js index 4c134d3f..da0f05dc 100644 --- a/static/boot.js +++ b/static/boot.js @@ -1848,7 +1848,7 @@ function applyBotName(){ applyEmptyStateSuggestionPref(); window._showTps=!!s.show_tps; window._fadeTextEffect=!!s.fade_text_effect; - window._showCliSessions=!!s.show_cli_sessions; + window._showCliSessions=s.show_cli_sessions!==false; window._showPreviousMessagingSessions=!!s.show_previous_messaging_sessions; window._soundEnabled=!!s.sound_enabled; window._notificationsEnabled=!!s.notifications_enabled; @@ -1959,7 +1959,7 @@ function applyBotName(){ applyEmptyStateSuggestionPref(); window._showTps=false; window._fadeTextEffect=false; - window._showCliSessions=false; + window._showCliSessions=true; // settings-load failed: mirror the True config default (#3988) window._soundEnabled=false; window._notificationsEnabled=false; window._whatsNewSummaryEnabled=false; diff --git a/static/panels.js b/static/panels.js index 4786db79..1e5dc259 100644 --- a/static/panels.js +++ b/static/panels.js @@ -6773,7 +6773,7 @@ async function loadSettingsPanel(){ const apiRedactCb=$('settingsApiRedact'); if(apiRedactCb){apiRedactCb.checked=settings.api_redact_enabled!==false;apiRedactCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const showCliCb=$('settingsShowCliSessions'); - if(showCliCb){showCliCb.checked=!!settings.show_cli_sessions;showCliCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} + if(showCliCb){showCliCb.checked=settings.show_cli_sessions!==false;showCliCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const showCronCb=$('settingsShowCronSessions'); if(showCronCb){ showCronCb.checked=!!settings.show_cron_sessions; diff --git a/tests/test_issue3988_show_cli_sessions_default.py b/tests/test_issue3988_show_cli_sessions_default.py new file mode 100644 index 00000000..7909202e --- /dev/null +++ b/tests/test_issue3988_show_cli_sessions_default.py @@ -0,0 +1,175 @@ +"""show_cli_sessions default-True + grandfather migration + hydration consistency (#3988). + +The default for ``show_cli_sessions`` flipped to **True** so that NEW installs +surface CLI / TUI / Telegram / Discord / Hermes One sessions in the WebUI sidebar +without users having to discover the toggle in Settings (the surprise described in +#3988). + +Two correctness properties this pins: + +1. **Grandfather established installs OFF.** An existing user who already completed + onboarding and never opted in must NOT have their sidebar silently change. The + ``load_settings()`` backfill pins ``show_cli_sessions=False`` when the saved + settings predate the flip (``onboarding_completed`` True, key absent). Fresh + installs (no settings file / onboarding not completed) get the True default. + +2. **Default-true hydration at EVERY read site.** config.py, boot.js, and the + panels.js Settings checkbox must all default an *absent* value to True — a + ``!!value`` coerce would default the True setting OFF for a user with no saved + value (the #4006 default-mismatch class), showing config-ON-but-UI-OFF. + +Combines + supersedes the approaches in #3997 (rodboev) and #4222 (Sanjays2402). +""" + +import json +import pathlib + +import pytest + +import api.config as config + +ROOT = pathlib.Path(__file__).resolve().parent.parent + + +def _read(rel): + return (ROOT / rel).read_text(encoding="utf-8") + + +# ── 1. config.py default ────────────────────────────────────────────────── + +def test_show_cli_sessions_default_is_true_in_config(): + assert config._SETTINGS_DEFAULTS.get("show_cli_sessions") is True, ( + "show_cli_sessions must default to True so CLI/TUI/messaging sessions are " + "visible by default for new installs (#3988)" + ) + + +def test_show_cli_sessions_in_bool_keys(): + assert "show_cli_sessions" in config._SETTINGS_BOOL_KEYS, ( + "show_cli_sessions must be in _SETTINGS_BOOL_KEYS so it round-trips as a bool" + ) + + +# ── 2. Grandfather migration in load_settings() ─────────────────────────── + +@pytest.fixture() +def settings_file(tmp_path, monkeypatch): + """Point config.SETTINGS_FILE at a temp file for isolated load_settings tests.""" + f = tmp_path / "settings.json" + monkeypatch.setattr(config, "SETTINGS_FILE", f) + return f + + +def _write(f, data): + f.write_text(json.dumps(data), encoding="utf-8") + + +def test_fresh_install_no_settings_file_defaults_on(settings_file): + """No settings file at all (brand-new install) → True (the new default).""" + assert not settings_file.exists() + loaded = config.load_settings() + assert loaded["show_cli_sessions"] is True + + +def test_fresh_install_onboarding_not_completed_defaults_on(settings_file): + """Settings file with ONLY a not-yet-completed onboarding flag (still + mid-first-run, no other user state) → True.""" + _write(settings_file, {"onboarding_completed": False}) + loaded = config.load_settings() + assert loaded["show_cli_sessions"] is True + + +def test_established_cli_user_without_onboarding_flag_grandfathered_off(settings_file): + """A CLI-configured user who tweaked a WebUI setting (e.g. theme) but never + ran the onboarding wizard is still an ESTABLISHED install — grandfather OFF. + + Keying the grandfather only on onboarding_completed would miss this user and + silently flip their sidebar on (Opus #4230 review finding). Any persisted + setting other than a falsy onboarding flag marks the install established. + """ + _write(settings_file, {"onboarding_completed": False, "theme": "dark"}) + loaded = config.load_settings() + assert loaded["show_cli_sessions"] is False, ( + "an install with persisted user settings (even without onboarding " + "completed) must be grandfathered OFF, not flipped on" + ) + + +def test_established_install_absent_key_grandfathered_off(settings_file): + """Established install (onboarding done) that never set the key → stays OFF. + + This is the core grandfather guarantee: an existing user's sidebar must not + silently start showing CLI/messaging sessions after the default flips. + """ + _write(settings_file, {"onboarding_completed": True, "theme": "dark"}) + loaded = config.load_settings() + assert loaded["show_cli_sessions"] is False, ( + "an onboarding-completed install with no saved show_cli_sessions must be " + "grandfathered OFF, not flipped on by the new default" + ) + + +def test_explicit_true_is_respected(settings_file): + """A user who explicitly opted in keeps it ON.""" + _write(settings_file, {"onboarding_completed": True, "show_cli_sessions": True}) + loaded = config.load_settings() + assert loaded["show_cli_sessions"] is True + + +def test_explicit_false_is_respected(settings_file): + """A user who explicitly turned it off keeps it OFF.""" + _write(settings_file, {"onboarding_completed": True, "show_cli_sessions": False}) + loaded = config.load_settings() + assert loaded["show_cli_sessions"] is False + + +def test_established_install_persists_grandfather_through_save(settings_file): + """save_settings() does load_settings() first, so the grandfathered False is + carried into the persisted blob — no window where a save flips an established + user ON. (save_settings writes the full merged dict.)""" + _write(settings_file, {"onboarding_completed": True}) + # A save that doesn't mention show_cli_sessions must not flip it on. + config.save_settings({"theme": "light"}) + persisted = json.loads(settings_file.read_text(encoding="utf-8")) + assert persisted.get("show_cli_sessions") is False, ( + "save_settings must preserve the grandfathered OFF state for established " + "installs, never silently flip it on" + ) + + +# ── 3. Default-true hydration at every frontend read site ────────────────── +# (string-pins; guard against the #4006 !!-coerce default-mismatch class) + +def test_boot_hydration_defaults_true_when_setting_absent(): + src = _read("static/boot.js") + assert "window._showCliSessions=s.show_cli_sessions!==false" in src, ( + "boot.js must default _showCliSessions True when the saved value is absent" + ) + assert "window._showCliSessions=!!s.show_cli_sessions" not in src, ( + "boot.js must not use !!s.show_cli_sessions — that defaults the True " + "setting OFF for users with no saved value (#4006 mismatch class)" + ) + + +def test_boot_settings_load_failure_fallback_defaults_true(): + """The settings-load-FAILED catch block must also default _showCliSessions + True, mirroring the config default — otherwise a transient settings-read + error silently hides CLI sessions (the #4006 catch-block-fallback class the + autoScrollFollow fix pinned).""" + src = _read("static/boot.js") + assert "window._showCliSessions=false" not in src, ( + "the settings-load-failure fallback must not hardcode _showCliSessions " + "false — it should mirror the True default" + ) + + +def test_settings_checkbox_renders_checked_by_default(): + src = _read("static/panels.js") + assert "showCliCb.checked=settings.show_cli_sessions!==false" in src, ( + "the show-CLI-sessions checkbox must default checked (!== false), matching " + "the True config default" + ) + assert "showCliCb.checked=!!settings.show_cli_sessions" not in src, ( + "panels.js must not use !!settings.show_cli_sessions for the checkbox — " + "that renders it unchecked by default, contradicting the config default" + ) diff --git a/tests/test_workspace_symlink_delete_rename.py b/tests/test_workspace_symlink_delete_rename.py new file mode 100644 index 00000000..ba1d294f --- /dev/null +++ b/tests/test_workspace_symlink_delete_rename.py @@ -0,0 +1,252 @@ +"""Regression tests for #4217 — symlink guards on /api/file/delete and /api/file/rename.""" +import os + +import pytest + +import api.routes as routes + + +@pytest.fixture() +def workspace(tmp_path): + """Yield a real temporary workspace directory and clean up after.""" + ws = tmp_path / "ws" + ws.mkdir() + yield ws + + +class _FakeHandler: + pass + + +def _patch_routes(ws): + """Monkey-patch routes helpers so handlers run without an HTTP server.""" + + class _S: + workspace = str(ws) + + cap = {} + orig_j = routes.j + orig_bad = routes.bad + orig_get = routes.get_session_for_file_ops + routes.j = lambda h, o: (cap.__setitem__("ok", o), True)[1] + routes.bad = lambda h, m, c=400: (cap.__setitem__("bad", (m, c)), True)[1] + routes.get_session_for_file_ops = lambda sid: _S() + return cap, (orig_j, orig_bad, orig_get) + + +def _restore_routes(originals): + routes.j, routes.bad, routes.get_session_for_file_ops = originals + + +# ── DELETE symlink guards ──────────────────────────────────────────────── + +def test_delete_workspace_symlink_to_dir_rejected(workspace): + real_dir = workspace / "real_dir" + real_dir.mkdir() + link = workspace / "link_to_dir" + try: + os.symlink(str(real_dir), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_delete( + _FakeHandler(), + {"session_id": "x", "path": "link_to_dir", "recursive": True}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot delete a symlinked entry" in cap["bad"][0] + assert real_dir.exists(), "real target directory was deleted through the symlink" + + +def test_delete_workspace_symlink_to_file_rejected(workspace): + real_file = workspace / "real_file.txt" + real_file.write_text("important") + link = workspace / "link_to_file.txt" + try: + os.symlink(str(real_file), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_delete( + _FakeHandler(), + {"session_id": "x", "path": "link_to_file.txt"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot delete a symlinked entry" in cap["bad"][0] + assert real_file.exists(), "real target file was deleted through the symlink" + + +# ── RENAME symlink guards ─────────────────────────────────────────────── + +def test_rename_workspace_symlink_to_dir_rejected(workspace): + real_dir = workspace / "real_dir" + real_dir.mkdir() + link = workspace / "link_to_dir" + try: + os.symlink(str(real_dir), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_rename( + _FakeHandler(), + {"session_id": "x", "path": "link_to_dir", "new_name": "renamed"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot rename a symlinked entry" in cap["bad"][0] + + +def test_rename_workspace_symlink_to_file_rejected(workspace): + real_file = workspace / "real_file.txt" + real_file.write_text("important") + link = workspace / "link_to_file.txt" + try: + os.symlink(str(real_file), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_rename( + _FakeHandler(), + {"session_id": "x", "path": "link_to_file.txt", "new_name": "renamed.txt"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot rename a symlinked entry" in cap["bad"][0] + + +# ── Non-symlink operations still work ─────────────────────────────────── + +def test_delete_real_dir_still_works(workspace): + real_dir = workspace / "deleteme" + real_dir.mkdir() + (real_dir / "child.txt").write_text("x") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_delete( + _FakeHandler(), + {"session_id": "x", "path": "deleteme", "recursive": True}, + ) + finally: + _restore_routes(orig) + + assert "ok" in cap, f"expected success, got {cap}" + assert cap["ok"]["ok"] is True + assert not real_dir.exists() + + +def test_rename_real_file_still_works(workspace): + real_file = workspace / "old_name.txt" + real_file.write_text("content") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_rename( + _FakeHandler(), + {"session_id": "x", "path": "old_name.txt", "new_name": "new_name.txt"}, + ) + finally: + _restore_routes(orig) + + assert "ok" in cap, f"expected success, got {cap}" + assert cap["ok"]["ok"] is True + assert not real_file.exists() + assert (workspace / "new_name.txt").exists() + + +# ── Existing move guard pinned ────────────────────────────────────────── + +def test_move_workspace_symlink_still_rejected(workspace): + real_file = workspace / "real.txt" + real_file.write_text("data") + dest = workspace / "dest" + dest.mkdir() + link = workspace / "link.txt" + try: + os.symlink(str(real_file), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_move( + _FakeHandler(), + {"session_id": "x", "path": "link.txt", "dest_dir": "dest"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot move a symlinked entry" in cap["bad"][0] + + +# ── Dangling symlinks: guard must fire BEFORE the exists() 404 (#4230 review) ── + +def test_delete_dangling_symlink_rejected_not_404(workspace): + """A dangling workspace symlink (target removed) must be rejected by the + symlink guard (400), not misclassified as 404 'File not found' and left + permanently undeletable. The is_symlink() check must precede exists().""" + link = workspace / "dangling_del" + try: + os.symlink(str(workspace / "gone_target"), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_delete( + _FakeHandler(), + {"session_id": "x", "path": "dangling_del", "recursive": True}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot delete a symlinked entry" in cap["bad"][0] + + +def test_rename_dangling_symlink_rejected_not_404(workspace): + """A dangling workspace symlink must be rejected by the rename guard (400), + not 404'd before the guard fires.""" + link = workspace / "dangling_ren" + try: + os.symlink(str(workspace / "gone_target"), str(link)) + except (OSError, NotImplementedError): + pytest.skip("platform does not support symlinks") + + cap, orig = _patch_routes(workspace) + try: + routes._handle_file_rename( + _FakeHandler(), + {"session_id": "x", "path": "dangling_ren", "new_name": "renamed"}, + ) + finally: + _restore_routes(orig) + + assert "bad" in cap, f"expected 400, got {cap}" + assert cap["bad"][1] == 400 + assert "Cannot rename a symlinked entry" in cap["bad"][0]