From fc589b689850523b318afd4dcdb4f9ae4fc6be92 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:09:25 +0530 Subject: [PATCH] fix(tui): one bound-profile backend for every workspace-cwd decision Follow-up to the #105749 + #123903 salvage: - A named profile without terminal.backend is local. Both contributor helpers fell back to the LAUNCH profile's backend, so a local profile opened from an ssh launch had its terminal.cwd treated as remote. - _bound_terminal_backend() is the single resolver (create, completion, workspace.move, display heal, settle-follow, terminal tool). The terminal tool now reads it too, so an ssh profile under a local launch keeps /home/kali instead of the display heal persisting /home. - _declared_remote_profile_cwd() only honours a profile that itself declares backend: ssh; the placeholder set is gone (the ~/absolute shape check already rejects ".", "auto", "cwd"). - One loader for a named profile's terminal section (_profile_terminal_cfg), shared with _profile_configured_cwd. - Eager row at session.create and "row cwd = explicit" on hydrate apply to remote sessions only; local project drafts stay lazy and keep settle-following, as on main. - config.get project forwards the pinned profile (and marks a picked path explicit) so the desktop gets the remote dir back; the renderer keeps adopting the server's normalized cwd for local users (WSL translation, abspath) instead of bypassing it. - Dropped #123903's cwd_explicit-decides-intent change in session.create: it made every local desktop new chat in a project lose its workspace and AGENTS.md. --- .../session/workspace-session-target.test.ts | 6 +- .../app/session/workspace-session-target.ts | 14 +-- tui_gateway/methods_config.py | 4 +- tui_gateway/methods_session.py | 16 ++-- tui_gateway/server.py | 41 +++++---- tui_gateway/session_workdir.py | 86 +++++-------------- 6 files changed, 62 insertions(+), 105 deletions(-) diff --git a/apps/desktop/src/app/session/workspace-session-target.test.ts b/apps/desktop/src/app/session/workspace-session-target.test.ts index 4f562e135c..0fc0cd911d 100644 --- a/apps/desktop/src/app/session/workspace-session-target.test.ts +++ b/apps/desktop/src/app/session/workspace-session-target.test.ts @@ -74,10 +74,8 @@ describe('startWorkspaceSession', () => { await second.promise await Promise.resolve() - // The explicitly chosen path is authoritative over the server-normalized cwd (a remote/ssh project - // dir is dropped to the launch cwd by host-side normalization); only the branch is adopted from the probe. - expect($newChatWorkspaceTarget.get()).toBe('/workspace-b') - expect($currentCwd.get()).toBe('/workspace-b') + expect($newChatWorkspaceTarget.get()).toBe('/normalized-b') + expect($currentCwd.get()).toBe('/normalized-b') expect($currentBranch.get()).toBe('main') }) diff --git a/apps/desktop/src/app/session/workspace-session-target.ts b/apps/desktop/src/app/session/workspace-session-target.ts index eb8b0b8e72..87527d2285 100644 --- a/apps/desktop/src/app/session/workspace-session-target.ts +++ b/apps/desktop/src/app/session/workspace-session-target.ts @@ -61,17 +61,19 @@ export function startWorkspaceSession({ const workspaceGeneration = $newChatWorkspaceTargetGeneration.get() setCurrentCwd(target) - void requestGateway<{ branch?: string; cwd?: string }>('config.get', { key: 'project', cwd: target }) + void requestGateway<{ branch?: string; cwd?: string }>('config.get', { + key: 'project', + cwd: target, + // The project's profile decides its terminal backend: an ssh project dir is not on this host, and + // resolving it under the launch profile would normalize it away to the launch cwd. + ...(profile ? { profile } : {}) + }) .then(info => { if ($newChatWorkspaceTargetGeneration.get() !== workspaceGeneration || activeSessionIdRef.current) { return } - // An explicitly chosen project path is authoritative (like the backend's explicit_cwd): a remote/ssh - // project dir does not exist on the gateway host, so config.get's host-side normalization drops it to - // the launch cwd (/opt/hermes). Keep the user's path; only adopt the server cwd for the path-less - // fallback. Branch still comes from the probe. - const resolved = explicitTarget || info.cwd || target + const resolved = info.cwd || target setCurrentCwd(resolved) setNewChatWorkspaceTarget(resolved) diff --git a/tui_gateway/methods_config.py b/tui_gateway/methods_config.py index a7597e0354..3a631c1872 100644 --- a/tui_gateway/methods_config.py +++ b/tui_gateway/methods_config.py @@ -191,7 +191,9 @@ def _cfg_get_provider(params): def _cfg_get_project(params): raw = str(params.get("cwd", "") or (_load_cfg().get("terminal") or {}).get("cwd", "") or "").strip() - cwd = _completion_cwd({"cwd": raw} if raw else {}) + # A picked path is explicit (the profile's terminal.cwd must not replace it); the profile picks the backend, + # so a remote project dir is kept instead of being dropped to the launch cwd by the host isdir check. + cwd = _completion_cwd({"cwd": raw, "cwd_explicit": bool(params.get("cwd")), "profile": params.get("profile")}) return {"cwd": cwd, "branch": git_probe.branch(cwd)} diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index 88a1aba7f3..d6b56df9e1 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -377,7 +377,7 @@ def _create_session(rid, params: dict, *, copy_parent_history: bool = False) -> raw_cwd = _str_param(params, "cwd") # unguarded, as on BASE: only the path check is best-effort # The BOUND profile's backend (multiplex hasn't rebound HERMES_HOME yet, so the process-global # _effective_terminal_backend() would read the launch profile - usually local). - session_backend = _profile_terminal_backend(profile_home) or _effective_terminal_backend() + session_backend = _bound_terminal_backend(profile_home) with contextlib.suppress(Exception): # A non-local backend's cwd lives inside the target environment, so a LOCAL isdir gate would # drop the remote project path and _terminal_task_cwd_with_source would fall to `~`. Mark it @@ -437,12 +437,10 @@ def _create_session(rid, params: dict, *, copy_parent_history: bool = False) -> _seed_branch_row(_sessions[sid], key, parent_session_id, history, source, profile_home) elif history: _seed_row(_sessions[sid]) - elif _sessions[sid].get("explicit_cwd"): - # A session opened INTO a project (explicit cwd) is explicit intent, not an abandoned draft, so the - # "no eager row" rule above does not apply: persist the row now with its project cwd. Otherwise the - # per-profile gateway process that runs the first turn mints the row itself (AIAgent INSERT-OR-IGNORE) - # with cwd=None -- the sidebar then drops the session to Home and the terminal falls back to the - # profile's ~ dir. Gated on explicit_cwd so plain launches still stay row-less (no "Untitled" litter). + elif explicit_cwd and session_backend != "local": + # A remote project session persists its row now: the per-profile gateway that runs the first turn mints + # the row itself (AIAgent INSERT-OR-IGNORE) with cwd=None, and the sidebar then drops it to Home. Local + # project drafts stay lazy — their cwd reaches the row on the first prompt (no "Untitled" litter). _ensure_session_db_row(_sessions[sid]) # Return immediately so Ink can paint; the AIAgent builds right after the flush. _schedule_agent_build(sid) @@ -1019,9 +1017,7 @@ def _(rid, params: dict) -> dict: # A non-local (ssh/docker) profile's workspace lives inside the target environment: the local isdir # gate would reject a valid remote project dir. Read the BOUND profile's backend (not the launch # process env) and, when non-local, trust the path raw - mirroring _completion_cwd / _set_session_cwd. - is_local = _profile_terminal_backend(_profile_home(params.get("profile"))) or _effective_terminal_backend() - is_local = is_local == "local" - if is_local: + if _bound_terminal_backend(_profile_home(params.get("profile"))) == "local": if not os.path.isdir(resolved): return _err(rid, 4017, f"working directory does not exist: {raw}") target_cwd = resolved diff --git a/tui_gateway/server.py b/tui_gateway/server.py index a2b53b0a58..56b291344c 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -619,34 +619,33 @@ def _profile_configured_cwd(profile_home: Path | None) -> str | None: env var (issue #40334). Returns an absolute, existing directory, or None for placeholders / missing / invalid paths. """ + return _configured_cwd_from_cfg({"terminal": _profile_terminal_cfg(profile_home)}) if profile_home else None + + +def _profile_terminal_cfg(profile_home: Path | str | None) -> dict: + """A named profile's ``terminal:`` section from ITS config.yaml ({} for the launch profile / fail-open).""" if profile_home is None: - return None + return {} with contextlib.suppress(Exception): from hermes_cli.config_effective import load_user_config_effective p = Path(profile_home) / "config.yaml" - return _configured_cwd_from_cfg(load_user_config_effective(p)) if p.exists() else None - return None + cfg = load_user_config_effective(p) if p.exists() else {} + terminal_cfg = cfg.get("terminal") if isinstance(cfg, dict) else None + return terminal_cfg if isinstance(terminal_cfg, dict) else {} + return {} -def _profile_terminal_backend(profile_home: Path | None) -> str | None: - """A non-launch profile's ``terminal.backend`` from ITS config.yaml (fail-open → None). +def _profile_terminal_backend(profile_home: Path | str | None) -> str | None: + """A named profile's ``terminal.backend`` from ITS config.yaml; None for the launch profile. Same reason as :func:`_profile_configured_cwd`: at ``session.create`` the multiplex gateway has NOT yet - rebound HERMES_HOME to the target profile, so ``_effective_terminal_backend()`` reads the LAUNCH profile - (usually ``local``). A session bound to an ``ssh``/``docker`` profile then loses the non-local cwd - exemption and its remote workspace is dropped to the launch dir. Read the bound profile's own backend. + rebound HERMES_HOME to the target profile, so ``_effective_terminal_backend()`` reads the LAUNCH profile. + A missing key is ``local`` (the default) — never the launch profile's backend, which would make a local + profile's paths look remote whenever the app was launched from an ssh profile. """ if profile_home is None: return None - with contextlib.suppress(Exception): - from hermes_cli.config_effective import load_user_config_effective - p = Path(profile_home) / "config.yaml" - if p.exists(): - cfg = load_user_config_effective(p) - terminal_cfg = cfg.get("terminal") if isinstance(cfg, dict) else None - if isinstance(terminal_cfg, dict): - return str(terminal_cfg.get("backend") or "").strip().lower() or None - return None + return str(_profile_terminal_cfg(profile_home).get("backend") or "").strip().lower() or "local" def _launch_configured_cwd() -> str | None: @@ -2599,11 +2598,11 @@ def _hydrate_session_cwd(sid: str, key: str, session_db, profile_home: str | Non if row and row.get("cwd"): with _sessions_lock: if sid in _sessions: - # A persisted row cwd is the session's authoritative workspace (a project session, or a - # settled dir), not a launch artifact: mark it explicit so the ssh/remote terminal uses it - # instead of falling back to the profile's ~ (session_workdir._terminal_task_cwd_with_source). _sessions[sid]["cwd"] = row["cwd"] - _sessions[sid]["explicit_cwd"] = True + # A remote session's stored cwd is its workspace: explicit, so the ssh/docker terminal uses + # it instead of the profile's ~. Local rows keep settle-following as before. + if not _session_is_local_backend(_sessions[sid]): + _sessions[sid]["explicit_cwd"] = True elif hasattr(db, "update_session_cwd"): try: _persist_session_cwd_and_schedule_git_meta(_sessions[sid], _sessions[sid]["cwd"], db=db) diff --git a/tui_gateway/session_workdir.py b/tui_gateway/session_workdir.py index 093c6bf594..83b20a2af9 100644 --- a/tui_gateway/session_workdir.py +++ b/tui_gateway/session_workdir.py @@ -34,10 +34,8 @@ def _completion_cwd(params: dict | None = None) -> str: profile_cwd = _profile_configured_cwd(profile_home) if profile_cwd: return profile_cwd - # SSH cwd usually does not exist on the desktop host, so the isdir - # check above drops it and the launch profile's workspace wins. - remote_cwd = _declared_remote_profile_cwd(profile_home) - if remote_cwd: + # An ssh profile's terminal.cwd is not on this host, so the isdir check above drops it. + if remote_cwd := _declared_remote_profile_cwd(profile_home): return remote_cwd # A session bound to another profile resolves its workspace from THAT profile's config before the launch profile's # env var; the dashboard's in-memory gateway does NOT inherit the PTY child's bridged TERMINAL_CWD, so a configured @@ -48,20 +46,14 @@ def _completion_cwd(params: dict | None = None) -> str: # The BOUND profile's backend, not the launch profile's: under multiplex HERMES_HOME is not yet rebound at # session.create, so the process-global _effective_terminal_backend() would misread an ssh/docker profile as # local and drop its remote cwd to getcwd(). - backend = _profile_terminal_backend(_profile_home(params.get("profile"))) or _effective_terminal_backend() + # A non-local backend's cwd lives inside the target environment, not on the host: pass it raw (as + # _terminal_task_cwd_with_source does) instead of the local isdir gate that would drop it to getcwd(). + if _bound_terminal_backend(_profile_home(params.get("profile"))) != "local": + return str(raw) with contextlib.suppress(Exception): resolved = os.path.abspath(os.path.expanduser(str(raw))) - # A non-local backend's cwd lives inside the target environment, not on the host: mirror the - # exemption _terminal_task_cwd_with_source already has (:58/:65) and pass it raw, skipping the - # local isdir gate that would otherwise drop the remote project path to getcwd(). - if backend != "local": - return resolved if os.path.isdir(resolved): return resolved - if not params.get("cwd_explicit"): - remote_cwd = _declared_remote_profile_cwd(_profile_home(params.get("profile"))) - if remote_cwd: - return remote_cwd return os.getcwd() @@ -74,46 +66,23 @@ def _workdir_terminal_cfg(key: str) -> str: return "" -_REMOTE_CWD_PLACEHOLDERS = {".", "./", "auto", "cwd"} - - -def _profile_terminal_section(profile_home) -> dict: - """``terminal:`` from a profile's own config.yaml, or {}.""" - if not profile_home: - return {} - with contextlib.suppress(Exception): - from pathlib import Path - - from hermes_cli.config_effective import load_user_config_effective - - path = Path(profile_home) / "config.yaml" - if not path.is_file(): - return {} - cfg = load_user_config_effective(path) - terminal = cfg.get("terminal") if isinstance(cfg, dict) else None - return terminal if isinstance(terminal, dict) else {} - return {} +def _bound_terminal_backend(profile_home) -> str: + """Terminal backend of the profile a session/RPC is bound to: a named profile's own config, else the process's.""" + return _profile_terminal_backend(Path(profile_home) if profile_home else None) or _effective_terminal_backend() def _declared_remote_profile_cwd(profile_home) -> str | None: - """A non-local profile's ``terminal.cwd``, kept when the path is not on this host. + """A named ssh profile's own ``terminal.cwd`` (``~``, ``~/…`` or absolute), unchecked against this host. - ``_profile_configured_cwd`` requires ``os.path.isdir``. An SSH working - directory lives on the remote, so that check drops it and the desktop - keeps using the launch profile's ``TERMINAL_CWD``. + ``_profile_configured_cwd`` requires ``os.path.isdir``; an ssh working directory lives on the remote, so that + check drops it and the launch profile's ``TERMINAL_CWD`` wins. Only a profile that itself declares + ``backend: ssh`` qualifies — placeholders (``.``/``auto``/``cwd``) fail the shape check. """ - terminal = _profile_terminal_section(profile_home) - if not terminal: - return None - backend = str(terminal.get("backend") or "").strip().lower() or _effective_terminal_backend() - if not backend or backend == "local": + terminal = _profile_terminal_cfg(profile_home) + if str(terminal.get("backend") or "").strip().lower() != "ssh": return None raw = str(terminal.get("cwd") or "").strip() - if not raw or raw in _REMOTE_CWD_PLACEHOLDERS: - return None - if raw == "~" or raw.startswith("~/") or os.path.isabs(raw): - return raw - return None + return raw if raw == "~" or raw.startswith("~/") or os.path.isabs(raw) else None def _terminal_task_cwd(session: dict | None) -> str: @@ -126,17 +95,14 @@ def _terminal_task_cwd_with_source(session: dict | None) -> tuple[str, str]: """``(cwd, source)``: ``"session"`` for THIS session's workspace (``explicit_cwd``/tracked dir), ``"process"`` for the global ``TERMINAL_CWD``/``terminal.cwd`` fallback — under per-session docker isolation that is a PREVIOUS session's launch artifact, so terminal_tool refuses it as a bind-mount source.""" - backend = _effective_terminal_backend() + backend = _bound_terminal_backend((session or {}).get("profile_home")) if backend != "local": # THIS session's explicit workspace beats the LAST session's env var. if session and session.get("explicit_cwd") and session.get("cwd"): return str(session["cwd"]), "session" - # Process TERMINAL_CWD is the launch profile. A named SSH profile's - # terminal.cwd is on the remote and must not lose to that env var. - if backend == "ssh": - remote_cwd = _declared_remote_profile_cwd((session or {}).get("profile_home")) - if remote_cwd: - return remote_cwd, "session" + # Process TERMINAL_CWD is the launch profile's; a named ssh profile's own terminal.cwd wins over it. + if remote_cwd := _declared_remote_profile_cwd((session or {}).get("profile_home")): + return remote_cwd, "session" raw = os.environ.get("TERMINAL_CWD", "").strip() or _workdir_terminal_cfg("cwd") if raw and raw not in {".", "auto", "cwd"}: return raw, "process" @@ -202,15 +168,9 @@ def _session_is_local_backend(session: dict | None) -> bool: for a session actually bound to an ssh/docker profile - which then heals its remote cwd to a host ancestor (/home) and persists that. Read the BOUND profile's backend first (profile_home), falling back to the process env only when the session carries no profile.""" - profile_home = session.get("profile_home") if session else None - if profile_home: - backend = _profile_terminal_backend(Path(profile_home)) - if backend: - return backend == "local" - # Fallback must read env OR config (like _terminal_task_cwd_with_source's _effective_terminal_backend), not - # env alone: a per-profile gateway (hermes -p felix) sets terminal.backend=ssh in config but leaves TERMINAL_ENV - # unset, so the env-only check reported "local" and healed a live remote cwd (/home/felix/... -> /home). - return _effective_terminal_backend() == "local" + # The process fallback reads env OR config: a per-profile gateway (hermes -p felix) sets terminal.backend=ssh + # in config but leaves TERMINAL_ENV unset, so an env-only check would heal a live remote cwd to /home. + return _bound_terminal_backend(session.get("profile_home") if session else None) == "local" def _effective_terminal_backend() -> str: