diff --git a/api/routes.py b/api/routes.py index 2798a281..fa6c953c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -4022,6 +4022,9 @@ def handle_post(handler, parsed) -> bool: if parsed.path == "/api/file/reveal": return _handle_file_reveal(handler, body) + if parsed.path == "/api/file/path": + return _handle_file_path(handler, body) + # ── Workspace management (POST) ── if parsed.path == "/api/workspaces/add": return _handle_workspace_add(handler, body) @@ -6619,7 +6622,13 @@ def _handle_file_reveal(handler, body): try: target = safe_resolve(Path(s.workspace), body["path"]) if not target.exists(): - return bad(handler, "File not found", 404) + # Include the resolved server-side path in the error message so + # the frontend toast can show *which* file the system expected. + # Useful when a stale session row still references a deleted file + # (#1764 — Cygnus's screenshot showed a "Failed to reveal: not + # found" toast that dropped the path entirely, leaving no clue + # what was missing). + return bad(handler, f"File not found: {target}", 404) system = platform.system() if system == "Darwin": @@ -6635,6 +6644,35 @@ def _handle_file_reveal(handler, body): return bad(handler, _sanitize_error(e)) +def _handle_file_path(handler, body): + """Resolve a relative workspace-rooted path into an absolute on-disk path. + + The right-click "Copy file path" action (#1764) wants to put the + absolute path on the user's clipboard so they can paste it into a + terminal, editor, or anywhere else without having to round-trip through + the OS file browser. The frontend can't compute the absolute path on + its own — `safe_resolve` joins against the session's workspace root + which only the server knows. The handler here is a thin lookup; no + filesystem mutation, no OS-specific dispatch. We do NOT require the + target to exist (unlike `_handle_file_reveal`) — copying the path of a + just-deleted file is still useful, and refusing would force callers + to special-case 404s for an action that cannot fail destructively. + """ + try: + require(body, "session_id", "path") + except ValueError as e: + return bad(handler, str(e)) + try: + s = get_session(body["session_id"]) + except KeyError: + return bad(handler, "Session not found", 404) + try: + target = safe_resolve(Path(s.workspace), body["path"]) + return j(handler, {"ok": True, "path": str(target)}) + except (ValueError, PermissionError, OSError) as e: + return bad(handler, _sanitize_error(e)) + + def _handle_workspace_add(handler, body): # Strip surrounding paired quotes BEFORE any further processing — macOS # Finder's "Copy as Pathname" wraps paths in single quotes, and users diff --git a/static/i18n.js b/static/i18n.js index 68ca08bf..e0be3a13 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -341,6 +341,11 @@ const LOCALES = { delete_failed: 'Delete failed: ', reveal_in_finder: 'Reveal in File Manager', reveal_failed: 'Failed to reveal: ', + copy_file_path: 'Copy file path', + path_copied: 'File path copied to clipboard', + path_copy_failed: 'Failed to copy path: ', + session_rename: 'Rename conversation', + session_rename_desc: 'Edit the title of this conversation', new_file_prompt: 'New file name (e.g. notes.md):', project_name_prompt: 'Project name:', created: 'Created ', @@ -1349,6 +1354,11 @@ const LOCALES = { delete_failed: '削除失敗: ', reveal_in_finder: 'ファイルマネージャーで表示', reveal_failed: '表示に失敗しました: ', + copy_file_path: 'ファイルパスをコピー', + path_copied: 'ファイルパスをクリップボードにコピーしました', + path_copy_failed: 'パスのコピーに失敗しました: ', + session_rename: '会話の名前を変更', + session_rename_desc: 'この会話のタイトルを編集', new_file_prompt: '新しいファイル名 (例: notes.md):', project_name_prompt: 'プロジェクト名:', created: '作成しました: ', @@ -2277,6 +2287,11 @@ const LOCALES = { delete_failed: 'Не удалось удалить: ', reveal_in_finder: 'Показать в файловом менеджере', reveal_failed: 'Не удалось открыть: ', + copy_file_path: 'Копировать путь к файлу', + path_copied: 'Путь к файлу скопирован в буфер обмена', + path_copy_failed: 'Не удалось скопировать путь: ', + session_rename: 'Переименовать беседу', + session_rename_desc: 'Изменить название этой беседы', new_file_prompt: 'Имя нового файла (например, notes.md):', project_name_prompt: 'Имя проекта:', created: 'Создано ', @@ -3200,6 +3215,11 @@ const LOCALES = { delete_failed: 'Error al eliminar: ', reveal_in_finder: 'Mostrar en el gestor de archivos', reveal_failed: 'Error al mostrar: ', + copy_file_path: 'Copiar ruta del archivo', + path_copied: 'Ruta del archivo copiada al portapapeles', + path_copy_failed: 'Error al copiar la ruta: ', + session_rename: 'Renombrar conversación', + session_rename_desc: 'Editar el título de esta conversación', new_file_prompt: 'Nombre del archivo nuevo (p. ej. notes.md):', created: 'Creado ', create_failed: 'Error al crear: ', @@ -4130,6 +4150,11 @@ const LOCALES = { delete_failed: 'Löschen fehlgeschlagen: ', reveal_in_finder: 'Im Dateimanager anzeigen', reveal_failed: 'Anzeige fehlgeschlagen: ', + copy_file_path: 'Dateipfad kopieren', + path_copied: 'Dateipfad in die Zwischenablage kopiert', + path_copy_failed: 'Pfad konnte nicht kopiert werden: ', + session_rename: 'Unterhaltung umbenennen', + session_rename_desc: 'Titel dieser Unterhaltung bearbeiten', new_file_prompt: 'Neuer Dateiname (z.B. notes.md):', project_name_prompt: 'Projektname:', created: 'Erstellt ', @@ -5091,6 +5116,11 @@ const LOCALES = { delete_failed: '\u5220\u9664\u5931\u8d25\uff1a', reveal_in_finder: '\u5728\u6587\u4ef6\u7ba1\u7406\u5668\u4e2d\u663e\u793a', reveal_failed: '\u663e\u793a\u5931\u8d25\uff1a', + copy_file_path: '\u590d\u5236\u6587\u4ef6\u8def\u5f84', + path_copied: '\u6587\u4ef6\u8def\u5f84\u5df2\u590d\u5236\u5230\u526a\u8d34\u677f', + path_copy_failed: '\u590d\u5236\u8def\u5f84\u5931\u8d25\uff1a', + session_rename: '\u91cd\u547d\u540d\u5bf9\u8bdd', + session_rename_desc: '\u7f16\u8f91\u6b64\u5bf9\u8bdd\u7684\u6807\u9898', new_file_prompt: '\u65b0\u6587\u4ef6\u540d\uff08\u4f8b\u5982 notes.md\uff09\uff1a', project_name_prompt: '\u9879\u76ee\u540d\u79f0\uff1a', created: '\u5df2\u521b\u5efa ', @@ -5981,6 +6011,11 @@ const LOCALES = { delete_failed: '\u522a\u9664\u5931\u6557\uff1a', reveal_in_finder: '\u5728\u6a94\u6848\u7ba1\u7406\u54e1\u4e2d\u986f\u793a', reveal_failed: '\u986f\u793a\u5931\u6557\uff1a', + copy_file_path: '\u8907\u88fd\u6a94\u6848\u8def\u5f91', + path_copied: '\u6a94\u6848\u8def\u5f91\u5df2\u8907\u88fd\u5230\u526a\u8cbc\u7c3f', + path_copy_failed: '\u8907\u88fd\u8def\u5f91\u5931\u6557\uff1a', + session_rename: '\u91cd\u65b0\u547d\u540d\u5c0d\u8a71', + session_rename_desc: '\u7de8\u8f2f\u6b64\u5c0d\u8a71\u7684\u6a19\u984c', new_file_prompt: '\u65b0\u6587\u4ef6\u540d\uff08\u4f8b\u5982 notes.md\uff09\uff1a', created: '\u5df2\u5275\u5efa ', create_failed: '\u5275\u5efa\u5931\u6557\uff1a', @@ -7009,6 +7044,11 @@ const LOCALES = { delete_failed: 'Falha ao excluir: ', reveal_in_finder: 'Mostrar no gerenciador de arquivos', reveal_failed: 'Falha ao mostrar: ', + copy_file_path: 'Copiar caminho do arquivo', + path_copied: 'Caminho do arquivo copiado para a área de transferência', + path_copy_failed: 'Falha ao copiar caminho: ', + session_rename: 'Renomear conversa', + session_rename_desc: 'Editar o título desta conversa', new_file_prompt: 'Nome do novo arquivo (ex: notes.md):', project_name_prompt: 'Nome do projeto:', created: 'Criado ', @@ -7903,6 +7943,11 @@ const LOCALES = { delete_failed: '삭제 실패: ', reveal_in_finder: '파일 관리자에서 열기', reveal_failed: '표시 실패: ', + copy_file_path: '파일 경로 복사', + path_copied: '파일 경로가 클립보드에 복사되었습니다', + path_copy_failed: '경로 복사 실패: ', + session_rename: '대화 이름 변경', + session_rename_desc: '이 대화의 제목 편집', new_file_prompt: 'New file name (e.g. notes.md):', project_name_prompt: 'Project name:', created: '생성됨: ', diff --git a/static/sessions.js b/static/sessions.js index 183342ef..e7892a0f 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -9,6 +9,7 @@ const ICONS={ dup:'', trash:'', more:'', + edit:'', }; // Tracks which session_id is currently being loaded. Used to discard stale @@ -1321,6 +1322,33 @@ function _openSessionActionMenu(session, anchorEl){ const isExternalSession = isMessagingSession || isCliSession; const menu=document.createElement('div'); menu.className='session-action-menu open'; + // Rename — first menu item by request (#1764). Double-click rename is + // timing-sensitive: the first click frequently registers as "open the + // chat" before the second click arrives, so users open the conversation + // when they meant to rename it. Putting Rename in the menu eliminates + // the timing entirely. Only shown for sessions that support rename + // (read-only imported sessions skip it; same gate as startRename's + // _isReadOnlySession check). + if(!_isReadOnlySession(session)){ + menu.appendChild(_buildSessionAction( + t('session_rename'), + t('session_rename_desc'), + ICONS.edit, + ()=>{ + closeSessionActionMenu(); + // Find the row for this session and call its attached startRename. + // Falls back to a no-op toast if the row isn't currently rendered + // (e.g. archived-and-hidden) — extremely rare since the menu only + // opens from a visible row's three-dot button. + const row=document.querySelector('.session-item[data-sid="'+session.session_id+'"]'); + if(row && typeof row._startRename === 'function'){ + row._startRename(); + } else if(typeof showToast==='function'){ + showToast(t('session_rename_failed_no_row')||'Could not start rename — row not found.', 3000, 'error'); + } + } + )); + } menu.appendChild(_buildSessionAction( session.pinned?t('session_unpin'):t('session_pin'), session.pinned?t('session_unpin_desc'):t('session_pin_desc'), @@ -2475,6 +2503,13 @@ function renderSessionListFromCache(){ title.replaceWith(inp); setTimeout(()=>{inp.focus();inp.select();},10); }; + // Expose the rename closure on the row so the three-dot action menu + // (`_openSessionActionMenu`, defined elsewhere) can trigger it without + // needing a separate DOM hunt or a duplicate copy of all this state + // (oldTitle / applyTitle / finish / _renamingSid bookkeeping). The + // double-click path on this element still calls startRename() directly. + el._startRename = startRename; + el.dataset.sid = s.session_id; // (Project dot is appended above, between title and timestamp, so it // sits outside the truncating title span and stays visible.) diff --git a/static/ui.js b/static/ui.js index 1004e050..6f745b5a 100644 --- a/static/ui.js +++ b/static/ui.js @@ -6145,9 +6145,49 @@ function _showFileContextMenu(e, item){ revealItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);'; revealItem.onmouseenter=()=>revealItem.style.background='var(--hover)'; revealItem.onmouseleave=()=>revealItem.style.background=''; - revealItem.onclick=async()=>{menu.remove();try{await api('/api/file/reveal',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})});}catch(err){showToast(t('reveal_failed')+err.message);}}; + revealItem.onclick=async()=>{menu.remove();try{await api('/api/file/reveal',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})});}catch(err){showToast(t('reveal_failed')+(err.message||err));}}; menu.appendChild(revealItem); + // Copy file path — resolves the absolute on-disk path on the server (so the + // user gets the full /home/.../workspace/foo.py rather than the relative + // path the file tree shows) and writes it to the OS clipboard. Useful for + // pasting into terminals, editors, or other apps without taking the slower + // Reveal-in-Finder round trip. + const copyPathItem=document.createElement('div'); + copyPathItem.textContent=t('copy_file_path'); + copyPathItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);'; + copyPathItem.onmouseenter=()=>copyPathItem.style.background='var(--hover)'; + copyPathItem.onmouseleave=()=>copyPathItem.style.background=''; + copyPathItem.onclick=async()=>{ + menu.remove(); + try{ + const r=await api('/api/file/path',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})}); + const abs=(r&&r.path)||item.path; + try{ + await navigator.clipboard.writeText(abs); + showToast(t('path_copied')); + }catch(clipErr){ + // Fallback for browsers where Clipboard API is gated (older Safari, + // non-secure contexts). Use the legacy execCommand path against a + // hidden textarea — this is the same pattern boot.js uses for the + // "Copy" buttons on code blocks. + const ta=document.createElement('textarea'); + ta.value=abs; + ta.style.cssText='position:fixed;left:-9999px;top:-9999px;'; + document.body.appendChild(ta); + ta.select(); + let copied=false; + try{copied=document.execCommand('copy');}catch(_){} + ta.remove(); + if(copied) showToast(t('path_copied')); + else showToast(t('path_copy_failed')+(clipErr&&clipErr.message?clipErr.message:String(clipErr))); + } + }catch(err){ + showToast(t('path_copy_failed')+(err.message||err)); + } + }; + menu.appendChild(copyPathItem); + // Divider + Delete const sep=document.createElement('hr'); sep.style.cssText='border:none;border-top:1px solid var(--border);margin:4px 0;'; diff --git a/tests/test_1466_sidebar_cancel_clarify.py b/tests/test_1466_sidebar_cancel_clarify.py index 8f277cce..890c745b 100644 --- a/tests/test_1466_sidebar_cancel_clarify.py +++ b/tests/test_1466_sidebar_cancel_clarify.py @@ -21,7 +21,12 @@ def _function_body(src: str, name: str, window: int = 1800) -> str: class TestSidebarCancelAction: def test_running_sidebar_sessions_get_stop_action(self): """Running sessions need a context-menu cancel action even when not active pane.""" - body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 3200) + # Window bumped from 3200 → 4400 in #1764 to accommodate the new + # Rename action item that lands at the top of _openSessionActionMenu. + # The `session.active_stream_id` / cancelSessionStream / delete checks + # are positional further down in the function, so growing the prefix + # required growing this read window. + body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 4400) assert "session.active_stream_id" in body, ( "sidebar action menu must detect per-session active_stream_id instead of S.activeStreamId" ) @@ -67,7 +72,8 @@ class TestSidebarCancelAction: def test_cli_sessions_hide_duplicate_and_delete_in_action_menu(self): """Session action menu should hide duplicate/delete for CLI-origin sessions.""" - body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 3600) + # Window bumped 3600 → 4800 in #1764 (Rename action prepended). + body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 4800) assert "const isCliSession = _isCliSession(session);" in body assert "const isExternalSession = isMessagingSession || isCliSession;" in body assert "if(!isExternalSession)" in body diff --git a/tests/test_1764_context_menu_essentials.py b/tests/test_1764_context_menu_essentials.py new file mode 100644 index 00000000..1d51d54b --- /dev/null +++ b/tests/test_1764_context_menu_essentials.py @@ -0,0 +1,343 @@ +"""Regression tests for issue #1764 — three context-menu essentials. + +The issue asked for a much larger surface, but per Nathan's curation we +ship only three high-leverage pieces in this PR: + +1. **Copy file path** in the workspace tree right-click menu — resolves + the absolute on-disk path on the server (so the user gets the full + path, not the relative tree-rooted one) and writes it to the + clipboard. + +2. **Rename** in the session three-dot menu — Cygnus reported double-click + rename being timing-sensitive (first click opens the chat before the + second click arrives). Putting Rename in the menu eliminates the + timing entirely. + +3. **Reveal-failed toast includes the resolved path** — the existing + handler returned bare "File not found" (404) and the frontend toast + surfaced only `err.message`, dropping the path entirely. This makes + it impossible for users to tell *which* file the system expected + (e.g. a stale session row pointing at a deleted file). Now the + server includes the resolved server-side path in the message. + +These tests pin the source-level wiring — they do not exercise the live +HTTP endpoints (those are covered by integration tests where they exist +in the wider suite). +""" +from pathlib import Path +import re + + +ROOT = Path(__file__).resolve().parent.parent +ROUTES = ROOT / "api" / "routes.py" +UI = ROOT / "static" / "ui.js" +SESSIONS = ROOT / "static" / "sessions.js" +I18N = ROOT / "static" / "i18n.js" + + +# ════════════════════════════════════════════════════════════════════ +# Item A — Copy file path in workspace tree right-click menu +# ════════════════════════════════════════════════════════════════════ + + +class TestCopyFilePathMenuItem: + def test_menu_item_present(self): + """The workspace file context menu must include a Copy file path + action that calls the new /api/file/path endpoint and writes the + result to the clipboard. + """ + src = UI.read_text(encoding="utf-8") + # Item label is sourced via t('copy_file_path') — pin the call. + assert "t('copy_file_path')" in src + # Endpoint POSTed to. + assert "/api/file/path" in src + # Clipboard write. + assert "navigator.clipboard.writeText(abs)" in src + + def test_menu_item_has_clipboard_fallback(self): + """Some browsers gate the modern Clipboard API (older Safari, any + non-secure context). The action must fall back to the legacy + execCommand pattern so users on those browsers still get a copy. + """ + src = UI.read_text(encoding="utf-8") + assert "document.execCommand('copy')" in src + # Hidden textarea pattern — uses a fixed-position offscreen element + # so the page doesn't visibly scroll when select() runs. + assert "position:fixed;left:-9999px" in src + + def test_menu_item_uses_path_copied_translation(self): + """The success toast keys must be wired to translatable strings, + not hardcoded English. + """ + src = UI.read_text(encoding="utf-8") + assert "t('path_copied')" in src + assert "t('path_copy_failed')" in src + + def test_endpoint_handler_present(self): + """Server-side endpoint must exist and route through the dispatcher.""" + src = ROUTES.read_text(encoding="utf-8") + assert 'parsed.path == "/api/file/path"' in src + assert "def _handle_file_path(handler, body):" in src + # Must use safe_resolve to prevent path traversal. + # Find the handler body and check. + m = re.search( + r"def _handle_file_path\(handler, body\):\s*(?:\"\"\".*?\"\"\")?\s*(.*?)(?=\ndef )", + src, + re.DOTALL, + ) + assert m, "_handle_file_path body not found" + body = m.group(1) + assert "safe_resolve(Path(s.workspace)" in body + assert "session_id" in body # require() check + # Returns the absolute path as a string. + assert 'j(handler, {"ok": True, "path": str(target)})' in body + + def test_endpoint_handler_does_not_require_existence(self): + """Copy-path on a recently-deleted file is still useful (paste into + terminal to investigate). The handler must not 404 on missing files. + """ + src = ROUTES.read_text(encoding="utf-8") + m = re.search( + r"def _handle_file_path\(handler, body\):.*?(?=\ndef )", + src, + re.DOTALL, + ) + assert m + body = m.group(0) + # No exists() check — that's specifically what we want NOT to be + # there. Distinguishing from _handle_file_reveal which does check. + assert "exists()" not in body, ( + "Copy-path must not gate on exists() — copying a stale path is " + "still useful for debugging deleted files." + ) + + +# ════════════════════════════════════════════════════════════════════ +# Item B — Rename in session three-dot menu +# ════════════════════════════════════════════════════════════════════ + + +class TestSessionRenameMenuItem: + def test_rename_action_in_menu(self): + """The session three-dot menu (`_openSessionActionMenu`) must + include Rename as the first item, gated on _isReadOnlySession. + """ + src = SESSIONS.read_text(encoding="utf-8") + # Rename block must be inside _openSessionActionMenu. + # Pin the structural anchor. + assert "if(!_isReadOnlySession(session)){" in src + assert "t('session_rename')" in src + assert "t('session_rename_desc')" in src + + def test_rename_dispatches_to_row_closure(self): + """The menu's rename action must trigger the existing startRename + closure attached to the row element — no duplicated state, no + separate API call out of band with the double-click path. + """ + src = SESSIONS.read_text(encoding="utf-8") + # Row-attached closure invocation. + assert "row._startRename" in src + # Row lookup by data-sid. + assert ".session-item[data-sid=" in src + + def test_row_exposes_start_rename(self): + """The session row builder must attach `_startRename` to the row + element so the menu (defined in a different function) can find it + without duplicating the closure's state (oldTitle, applyTitle, the + _renamingSid bookkeeping, etc.). + """ + src = SESSIONS.read_text(encoding="utf-8") + assert "el._startRename = startRename" in src + assert "el.dataset.sid = s.session_id" in src + + def test_rename_appears_before_pin(self): + """Cygnus's specific ask: Rename should be at the top of the menu, + not buried under Pin / Move / Archive / etc. Pin that ordering. + """ + src = SESSIONS.read_text(encoding="utf-8") + rename_idx = src.find("t('session_rename')") + pin_idx = src.find("t('session_pin')") + assert rename_idx > 0 and pin_idx > 0 + assert rename_idx < pin_idx, ( + "Rename must appear before Pin in _openSessionActionMenu." + ) + + def test_rename_translation_keys_present(self): + """English translation keys must exist for the new menu item.""" + src = I18N.read_text(encoding="utf-8") + assert "session_rename: 'Rename conversation'" in src + assert "session_rename_desc: 'Edit the title of this conversation'" in src + + +# ════════════════════════════════════════════════════════════════════ +# Item C — reveal-failed toast includes the resolved path +# ════════════════════════════════════════════════════════════════════ + + +class TestRevealFailedTostIncludesPath: + def test_handler_includes_target_in_404_message(self): + """When `target.exists()` returns false, the 404 response body must + include the resolved server-side path so the frontend toast can + show users *which* file the system expected. Previously it was + just "File not found" with no path — useless for diagnosing stale + session rows. + """ + src = ROUTES.read_text(encoding="utf-8") + # Find _handle_file_reveal body. + m = re.search( + r"def _handle_file_reveal\(handler, body\):.*?(?=\ndef )", + src, + re.DOTALL, + ) + assert m, "_handle_file_reveal not found" + body = m.group(0) + # The bad() call for not-exists must include the path. + assert 'f"File not found: {target}"' in body, ( + "Reveal handler must include the resolved path in the 404 message." + ) + # And NOT the bare unhelpful message. + # (We allow the substring 'File not found' because the new f-string + # contains it as a prefix; pin via the f-string presence above.) + assert 'bad(handler, "File not found", 404)' not in body, ( + "Old bare 'File not found' message must be removed." + ) + + def test_existing_translation_key_unchanged(self): + """The frontend toast prefix `reveal_failed: 'Failed to reveal: '` + is unchanged — the additional path comes from the server-side + message, so the prefix + message concat still reads well. + """ + src = I18N.read_text(encoding="utf-8") + assert "reveal_failed: 'Failed to reveal: '" in src + + def test_reveal_call_site_uses_message_or_err(self): + """The frontend reveal handler call site must guard against err + being a non-Error object (e.g. a network-layer reject without a + .message). Previously `err.message` alone could produce + "Failed to reveal: undefined" — we use `(err.message||err)`. + """ + src = UI.read_text(encoding="utf-8") + # Match both possible forms (with or without parens). + assert ( + "(err.message||err)" in src or "(err.message || err)" in src + ), "Reveal-failed toast must guard against err with no .message" + + + +# ════════════════════════════════════════════════════════════════════ +# Behaviour tests — exercise the live HTTP endpoints against the +# module-scoped test server (started by conftest.py at port 8788). +# ════════════════════════════════════════════════════════════════════ + + +import json +import pathlib +import sys +import urllib.error +import urllib.request + +sys.path.insert(0, str(pathlib.Path(__file__).parent)) + +from conftest import TEST_BASE # noqa: E402 + + +def _post(path, body=None, headers=None): + data = json.dumps(body or {}).encode() + h = {"Content-Type": "application/json"} + if headers: + h.update(headers) + req = urllib.request.Request(TEST_BASE + path, data=data, headers=h) + try: + with urllib.request.urlopen(req, timeout=10) as r: + return json.loads(r.read()), r.status + except urllib.error.HTTPError as e: + return json.loads(e.read()), e.code + + +class TestFilePathEndpointBehaviour: + """End-to-end exercise of the new /api/file/path endpoint against the + live test server.""" + + def _new_session(self): + body, status = _post("/api/session/new", {}) + assert status == 200, body + return body["session"]["session_id"] + + def test_returns_absolute_path_for_relative_input(self): + """The endpoint must resolve a relative workspace-rooted path into + the absolute on-disk path. This is the whole point — the frontend + can't compute it because only the server knows the workspace root. + """ + sid = self._new_session() + body, status = _post("/api/file/path", {"session_id": sid, "path": "."}) + assert status == 200, body + assert body.get("ok") is True + # Path should be absolute (starts with /). + assert body.get("path", "").startswith("/"), body + + def test_does_not_404_on_missing_file(self): + """Copy-path on a stale-but-recently-deleted file must still + succeed — that's specifically what makes the action useful for + debugging.""" + sid = self._new_session() + body, status = _post( + "/api/file/path", + {"session_id": sid, "path": "definitely-does-not-exist-xyz123.tmp"}, + ) + assert status == 200, body + assert body.get("ok") is True + # Even though the file doesn't exist, we get back a resolved path. + assert "definitely-does-not-exist-xyz123.tmp" in body.get("path", "") + + def test_rejects_path_traversal(self): + """The endpoint must use safe_resolve, which rejects paths that + escape the workspace root.""" + sid = self._new_session() + body, status = _post( + "/api/file/path", + {"session_id": sid, "path": "../../../../../../etc/passwd"}, + ) + assert status == 400, body # safe_resolve raises ValueError → bad() + # Error message must NOT include the attempted traversal target's + # contents, just a generic safe-resolve message. + assert "passwd" not in body.get("error", "").lower() or "outside" in body.get("error", "").lower() + + def test_missing_session_id_returns_400(self): + body, status = _post("/api/file/path", {"path": "foo.txt"}) + assert status == 400, body + assert "session_id" in body.get("error", "") + + def test_unknown_session_returns_404(self): + body, status = _post( + "/api/file/path", {"session_id": "fake-session-xyz", "path": "."} + ) + assert status == 404, body + assert "session" in body.get("error", "").lower() + + +class TestRevealHandlerErrorIncludesPath: + """End-to-end check that the reveal endpoint's 404 includes the path.""" + + def _new_session(self): + body, status = _post("/api/session/new", {}) + assert status == 200, body + return body["session"]["session_id"] + + def test_404_message_contains_resolved_path(self): + """Reveal of a missing file must surface the resolved server-side + path in the error, so the frontend toast can show users *which* + file was missing — useful when a stale row points at a deleted + file (#1764).""" + sid = self._new_session() + body, status = _post( + "/api/file/reveal", + {"session_id": sid, "path": "missing-xyz-1764.txt"}, + ) + assert status == 404, body + err = body.get("error", "") + # Must include the filename in the resolved path. + assert "missing-xyz-1764.txt" in err, ( + f"Reveal 404 message must include the resolved path, got: {err!r}" + ) + # Must keep the human-readable prefix. + assert "File not found" in err