diff --git a/project_docs/REMOTE_CLIENT_SHARES_IMPLEMENTATION_PHASES.md b/project_docs/REMOTE_CLIENT_SHARES_IMPLEMENTATION_PHASES.md index e5d0cf8..eb2b3ac 100644 --- a/project_docs/REMOTE_CLIENT_SHARES_IMPLEMENTATION_PHASES.md +++ b/project_docs/REMOTE_CLIENT_SHARES_IMPLEMENTATION_PHASES.md @@ -1,5 +1,17 @@ # Remote Client Shares Implementation Phases V1.1 +## Status + +Per huidige repositorystatus zijn de in dit document beschreven implementatiefases afgerond: + +- Phase 1: afgerond +- Phase 2: afgerond +- Phase 3: afgerond + +Dit document beschrijft geen Phase 4. + +De sectie `Later` hieronder blijft expliciet buiten de beschreven fasering van V1.1 en is geen impliciete volgende fase. + ## Doel Dit document splitst `REMOTE_CLIENT_SHARES_V1_DESIGN.md` op in pragmatische implementatiefases. @@ -32,6 +44,10 @@ Info, tekstpreview, eenvoudige image preview en download voor remote shares. Alle write-acties, bookmarks/startup paths en cross-source flows. +Opmerking: + +- `Later` betekent in dit document: bewust uitgestelde scope, niet een gedefinieerde volgende implementatiefase + --- ## Phase 1: Client Registry @@ -285,6 +301,11 @@ Nieuwe endpoints: Deze onderdelen horen niet in V1.1. +Status: + +- deze onderdelen blijven expliciet buiten de afgeronde Phase 1 t/m Phase 3 scope +- voor deze onderdelen bestaat in dit document geen aparte vervolgfase + ### Write-acties - mkdir diff --git a/project_docs/REMOTE_CLIENT_SHARES_V1_DESIGN.md b/project_docs/REMOTE_CLIENT_SHARES_V1_DESIGN.md index fc51acf..9432ded 100644 --- a/project_docs/REMOTE_CLIENT_SHARES_V1_DESIGN.md +++ b/project_docs/REMOTE_CLIENT_SHARES_V1_DESIGN.md @@ -1,5 +1,20 @@ # Remote Client Shares V1.1 Design +## Status + +Dit document beschrijft de V1.1-doelscope voor Remote Client Shares. + +Per huidige repositorystatus valt de beschreven V1.1 read-mostly scope onder afgeronde implementatie van: + +- client registry +- browse via `/Clients` +- file info +- tekstpreview +- eenvoudige image preview +- download + +De expliciet niet in V1.1 opgenomen onderdelen hieronder blijven buiten scope en vormen in dit document geen aparte vervolgfase. + ## Doel Een gebruiker van WebManager moet naast de bestaande server-side storage-roots ook een beperkte set lokale mappen van zijn eigen client-Mac kunnen benaderen, zonder de hele homefolder bloot te geven. @@ -88,6 +103,11 @@ Daarom mogen remote client shares niet in hetzelfde model worden gestopt als `ro - automatische LAN discovery - multi-user auth met OS user mapping +Status: + +- deze lijst blijft expliciet uitgesloten van V1.1 +- dit document definieert hiervoor geen Phase 4 of andere vervolgfase + --- ## Gewenste gebruikerservaring diff --git a/project_docs/research_fase_4.md b/project_docs/research_fase_4.md new file mode 100644 index 0000000..d1c596f --- /dev/null +++ b/project_docs/research_fase_4.md @@ -0,0 +1,194 @@ +# Research: Remote Single-File Copy To Host + +## Relevante file analysis + +### Backend + +- [routes_files.py](/workspace/webmanager-mvp/webui/backend/app/api/routes_files.py) + Bevat de bestaande lokale upload-route (`POST /api/files/upload`) en de remote read-only Phase 3 routes (`view`, `info`, `download`, `image`) via `RemoteFileService`. +- [routes_copy.py](/workspace/webmanager-mvp/webui/backend/app/api/routes_copy.py) + Bevat de bestaande copy-route (`POST /api/files/copy`) die volledig uitgaat van host-side source en host-side destination. +- [file_ops_service.py](/workspace/webmanager-mvp/webui/backend/app/services/file_ops_service.py) + Bevat lokale file-acties. Relevant is vooral `upload()`, omdat die host-write doet na `PathGuard`-validatie van een doeldirectory. +- [copy_task_service.py](/workspace/webmanager-mvp/webui/backend/app/services/copy_task_service.py) + Bevat task-opbouw, destination-validatie en taakcreatie voor copy, maar gaat uit van een lokale bron die via `PathGuard` naar een host-pad resolveert. +- [remote_file_service.py](/workspace/webmanager-mvp/webui/backend/app/services/remote_file_service.py) + Bevat al de benodigde remote read-path parsing, share-validatie via registry, agent-auth, error mapping en een gestreamde `prepare_download()` naar de agent. +- [filesystem_adapter.py](/workspace/webmanager-mvp/webui/backend/app/fs/filesystem_adapter.py) + Bevat de feitelijke host-write helpers: + - `write_uploaded_file(path, file_stream, overwrite=False)` + - `copy_file(source, destination, on_progress=None)` + `copy_file` vereist een lokale bron op de host en is dus niet bruikbaar voor remote input. `write_uploaded_file` schrijft een inkomende stream naar een hostpad en is conceptueel het dichtstbij. +- [path_guard.py](/workspace/webmanager-mvp/webui/backend/app/security/path_guard.py) + Houdt host-write validatie strikt lokaal. Dat moet zo blijven; remote paden mogen hier niet als bronsemantiek in terechtkomen. +- [tasks_runner.py](/workspace/webmanager-mvp/webui/backend/app/tasks_runner.py) + Bevat task-based copy/move uitvoering, maar alleen voor host-side bronpaden. Wel relevant als patroon voor een aparte remote-to-host worker. +- [schemas.py](/workspace/webmanager-mvp/webui/backend/app/api/schemas.py) + Bevat bestaande `CopyRequest` en upload/copy response-modellen. Voor een aparte feature is waarschijnlijk een nieuw requestmodel nodig. + +### Frontend + +- [app.js](/workspace/webmanager-mvp/webui/html/app.js) + Relevante bestaande flows: + - `uploadFileRequest()` gebruikt uitsluitend `/api/files/upload` + - `startCopySelected()` gebruikt uitsluitend `/api/files/copy` + - remote browse/view/download is al source-aware + - remote copy is nu bewust geblokkeerd + Dit bevestigt dat upload-flow en copy-flow momenteel twee losse UI-contracten zijn. + +### Agent + +- [finder_commander/app/main.py](/workspace/webmanager-mvp/finder_commander/app/main.py) + Agent heeft al wat voor deze feature nodig is: + - strikte `share + relative path` validatie + - `GET /api/info` + - `GET /api/download` + Voor remote single-file copy naar host is geen nieuwe remote write-API nodig. + +## Oordeel over hergebruik van upload-internals + +### Bestaande upload-functionaliteit aanpassen? + +Nee. + +Reden: + +- de bestaande upload-route, upload-requestvorm en upload-UI werken al goed +- upload is browser -> host via multipart/form-data +- de gewenste feature is agent/remote -> host via backend-proxy/stream +- dat is een ander contract, andere foutbron en andere bronsemantiek + +### Interne host-write logica hergebruiken? + +Ja, maar alleen op intern helper/service-niveau. + +Concreet oordeel: + +- `FilesystemAdapter.copy_file()` is niet geschikt voor hergebruik + Reden: vereist een lokale host-bronpad als source. +- `FilesystemAdapter.write_uploaded_file()` is deels relevant + Reden: dit doet precies de host-write van een inkomende stream naar een doelbestand. +- Direct hergebruik van `FileOpsService.upload()` is niet verstandig + Reden: die methode is semantisch en contractueel gekoppeld aan multipart upload en `UploadFile`. + +Best passende richting: + +- niet hergebruiken via bestaande upload-endpoints of upload-flow +- wel overwegen om de onderliggende stream-naar-bestand write logica te hergebruiken of te veralgemeniseren in `FilesystemAdapter` +- voorkeur: een nieuwe sibling-helper zoals `write_stream_file(...)` of een kleine interne extractie, zodat upload ongewijzigd blijft en remote copy dezelfde veilige host-write primitief kan gebruiken + +## Ontwerpvoorstel + +### Feature + +`Copy remote file to host` + +### Scope + +- alleen single file +- alleen source onder `/Clients/...` +- alleen destination op host-side lokale map +- geen mappen +- geen overwrite in eerste change request tenzij expliciet gewenst +- geen upload-route hergebruik +- geen brede refactor + +### Backendontwerp + +Voeg een aparte backend feature toe, niet via `POST /api/files/upload` en niet via bestaande `POST /api/files/copy`. + +Voorkeursvorm: + +- nieuwe route, bijvoorbeeld `POST /api/files/remote-copy` +- request bevat: + - `source`: remote bestandspad onder `/Clients/...` + - `destination_dir`: host-directory pad + +Nieuwe service, bijvoorbeeld: + +- `RemoteCopyToHostService` + +Verantwoordelijkheden: + +1. valideer dat `source` een remote `/Clients/...` file is +2. valideer dat `destination_dir` een host-directory is via bestaande lokale `PathGuard` +3. haal remote metadata op of resolve remote naam via bestaande `RemoteFileService` +4. bouw destination pad als `destination_dir/` +5. faal op bestaand doelbestand in eerste versie +6. open remote download-stream via aparte interne helper op `RemoteFileService` +7. schrijf gestreamd naar host met een aparte interne host-write helper +8. map fouten strikt: + - remote unavailable blijft lokale actie-fout + - host permission/path-conflict blijft gewone host-fout + +### Aanbevolen interne hergebruikslijn + +- laat `RemoteFileService` een interne streaming primitive aanbieden, bijvoorbeeld een variant op de huidige remote download-open logica zonder HTTP-response voor browser-download +- laat `FilesystemAdapter` een aparte stream-write helper aanbieden voor generieke inkomende streams +- laat upload zijn bestaande publieke route en flow behouden + +### Frontendontwerp + +Geen wijziging aan upload-UI. + +Kleine aparte UI-feature: + +- toon een aparte actie alleen als: + - bronpane een remote file-selectie heeft van exact 1 bestand + - doelpane op een host/local directory staat +- de actie roept de nieuwe backend-route aan +- na succes: + - refresh beide panes + - toon lokale foutmelding bij falen + +Voorkeur: + +- aparte actie of expliciete source-aware branch voor "Copy remote file to host" +- niet de bestaande upload-flow hergebruiken + +### Agentontwerp + +Geen nieuwe agent-endpoints nodig in deze scope. + +De bestaande `GET /api/download` is voldoende als read-only bron voor streaming. + +## Acceptance criteria + +- een enkel bestand onder `/Clients/...` kan naar een host-directory worden gekopieerd +- de destination moet een host/local directory zijn +- mappen als remote bron worden geweigerd +- remote -> remote wordt geweigerd +- host -> remote wordt geweigerd +- overwrite gebeurt niet impliciet; bestaand doelbestand geeft een nette fout +- bestaande upload-route, upload-contract en upload-UI blijven ongewijzigd +- bestaande lokale copy-flow blijft ongewijzigd +- remote fouten blijven lokaal tot deze actie +- host-write blijft onder bestaande lokale `PathGuard`-regels vallen +- data wordt gestreamd; geen volledige file-buffer in memory + +## Klein plan + +1. Voeg een research-backed change request toe voor een aparte route `POST /api/files/remote-copy`. +2. Voeg een kleine service toe die alleen remote single-file source + local destination_dir ondersteunt. +3. Voeg een interne streaming helper toe in `RemoteFileService` voor remote bestand-inname door backend. +4. Voeg een aparte interne host-write helper toe in `FilesystemAdapter` voor generieke stream-naar-bestand writes, zonder upload-API te wijzigen. +5. Voeg minimale frontend wiring toe voor een aparte "Copy remote file to host"-actie. +6. Test stapsgewijs: + - success path remote file -> local dir + - bestaand doelbestand + - remote directory rejected + - remote failure stays local + - upload-regressie: bestaande `/api/files/upload` blijft ongewijzigd + +## Expliciete lijst van wat buiten scope blijft + +- remote mappen kopiëren +- remote write-acties +- remote -> remote +- host -> remote +- aanpassing van bestaande upload-routes +- aanpassing van upload-requestcontract +- aanpassing van upload-UI +- brede refactor van copy/upload/task-infrastructuur +- bookmarks/startup paths +- remote task-runner verbreding buiten deze ene actie diff --git a/webui/backend/data/tasks.db b/webui/backend/data/tasks.db index 27b01ec..5bb2c16 100644 Binary files a/webui/backend/data/tasks.db and b/webui/backend/data/tasks.db differ diff --git a/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc index 640885b..0995f63 100644 Binary files a/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc and b/webui/backend/tests/golden/__pycache__/test_ui_smoke_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/test_ui_smoke_golden.py b/webui/backend/tests/golden/test_ui_smoke_golden.py index a67fea6..6c9fc66 100644 --- a/webui/backend/tests/golden/test_ui_smoke_golden.py +++ b/webui/backend/tests/golden/test_ui_smoke_golden.py @@ -850,7 +850,7 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('id="download-modal-close-btn"', body) self.assertIn('id="context-menu"', body) self.assertIn('id="context-menu-scope"', body) - self.assertIn('id="context-menu-target"', body) + self.assertNotIn('id="context-menu-target"', body) self.assertIn('id="context-menu-open-btn"', body) self.assertIn('id="context-menu-edit-btn"', body) self.assertIn('id="context-menu-download-btn"', body) @@ -1133,7 +1133,7 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('setStatus("Preparing download...");', app_js) self.assertIn('setStatus("Requesting download...");', app_js) self.assertIn('setStatus(zipDownload ? "Preparing download..." : "Requesting download...");', app_js) - self.assertIn('setStatus(`Download requested: ${anchor.download}`);', app_js) + self.assertIn('setStatus(`Download requested: ${fileName}`);', app_js) self.assertIn('"/api/files/download/archive-prepare"', app_js) self.assertIn('"/api/files/duplicate"', app_js) self.assertIn('"/api/files/delete"', app_js) @@ -1181,24 +1181,26 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('if (!row) {', app_js) self.assertIn('closeContextMenu();', app_js) self.assertIn('elements.openButton.classList.toggle("hidden", isMulti);', app_js) - self.assertIn('const openableSingle = items.length === 1 && isOpenableSelection(items[0]);', app_js) + self.assertIn('const openableSingle =', app_js) + self.assertIn('items[0].kind === "directory" || isRemoteViewableSelection(items[0])', app_js) self.assertIn('elements.openButton.disabled = !openableSingle;', app_js) self.assertIn('if (item.kind === "directory") {', app_js) self.assertIn('return isImageSelection(item) || isVideoSelection(item);', app_js) - self.assertIn('const editableSingle = items.length === 1 && isEditableSelection(items[0]);', app_js) + self.assertIn('const editableSingle = items.length === 1 && !remoteSelection && isEditableSelection(items[0]);', app_js) self.assertIn('return [".txt", ".log", ".md", ".yml", ".yaml", ".json", ".js", ".py", ".css", ".html", ".conf"].some((suffix) => lower.endsWith(suffix));', app_js) self.assertIn('if (!item || item.kind !== "file") {', app_js) - self.assertIn('elements.editButton.classList.toggle("hidden", isMulti || items.length !== 1 || items[0].kind !== "file");', app_js) + self.assertIn('elements.editButton.classList.toggle("hidden", isMulti || items.length !== 1 || items[0].kind !== "file" || remoteSelection);', app_js) self.assertIn('elements.editButton.disabled = !editableSingle;', app_js) - self.assertIn('const downloadableSelection = items.length > 0;', app_js) + self.assertIn('const downloadableSelection = items.length === 1 && items[0].kind === "file";', app_js) self.assertIn('elements.downloadButton.classList.remove("hidden");', app_js) self.assertIn('elements.downloadButton.disabled = !downloadableSelection;', app_js) - self.assertIn('elements.renameButton.classList.toggle("hidden", isMulti);', app_js) + self.assertIn('elements.renameButton.classList.toggle("hidden", isMulti || remoteSelection);', app_js) self.assertIn('elements.duplicateButton.classList.remove("hidden");', app_js) - self.assertIn('elements.duplicateButton.disabled = items.length === 0;', app_js) + self.assertIn('elements.duplicateButton.disabled = remoteSelection || items.length === 0;', app_js) self.assertIn('elements.copyButton.classList.remove("hidden");', app_js) - self.assertIn('elements.copyButton.disabled = items.length === 0;', app_js) + self.assertIn('elements.copyButton.disabled = remoteSelection || items.length === 0;', app_js) self.assertIn('elements.moveButton.classList.remove("hidden");', app_js) + self.assertIn('elements.moveButton.disabled = remoteSelection || items.length === 0;', app_js) self.assertIn('elements.propertiesButton.classList.remove("hidden");', app_js) self.assertIn('elements.propertiesButton.disabled = items.length === 0;', app_js) self.assertIn('openCurrentDirectory();', app_js) @@ -1207,8 +1209,8 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('const created = await createArchiveDownloadTask(selectedPaths);', app_js) self.assertIn('const task = await waitForArchiveDownloadReady(created.task_id);', app_js) self.assertIn('startArchiveDownload(task.id, task.destination);', app_js) - self.assertIn('const { blob, fileName } = await downloadFileRequest(selectedPaths);', app_js) - self.assertIn('anchor.download = fileName || selected.name;', app_js) + self.assertIn('const response = await downloadFileRequest(selectedPaths);', app_js) + self.assertIn('anchor.download = response.fileName || selected.name;', app_js) self.assertIn('openRenamePopup();', app_js) self.assertIn('const result = await createDuplicateTask(selectedItems.map((item) => item.path));', app_js) self.assertIn('showActionSummary("Duplicate", 1, 0, null);', app_js) @@ -1233,7 +1235,7 @@ class UiSmokeGoldenTest(unittest.TestCase): self.assertIn('renderInfoField("Selected items", selectedItems.length);', app_js) self.assertIn('renderInfoField("Files", fileCount);', app_js) self.assertIn('renderInfoField("Directories", directoryCount);', app_js) - self.assertIn('document.getElementById("copy-btn").disabled = !hasSelection;', app_js) + self.assertIn('document.getElementById("copy-btn").disabled = remoteBrowse || !hasSelection;', app_js) self.assertNotIn('Only files are supported for copy', app_js) self.assertIn('document.getElementById("upload-menu-toggle").onclick = (event) => {', app_js) self.assertIn('document.getElementById("upload-folder-btn").onclick = openFolderPicker;', app_js) diff --git a/webui/html/app.js b/webui/html/app.js index 7cc282f..3f97ecb 100644 --- a/webui/html/app.js +++ b/webui/html/app.js @@ -436,7 +436,6 @@ function contextMenuElements() { return { menu: document.getElementById("context-menu"), scope: document.getElementById("context-menu-scope"), - target: document.getElementById("context-menu-target"), openButton: document.getElementById("context-menu-open-btn"), editButton: document.getElementById("context-menu-edit-btn"), downloadButton: document.getElementById("context-menu-download-btn"), @@ -774,7 +773,8 @@ function closeContextMenu() { } elements.menu.classList.add("hidden"); elements.scope.textContent = ""; - elements.target.textContent = ""; + elements.menu.style.left = ""; + elements.menu.style.top = ""; } function openContextMenu(pane, entry, event) { @@ -800,7 +800,6 @@ function openContextMenu(pane, entry, event) { const editableSingle = items.length === 1 && !remoteSelection && isEditableSelection(items[0]); const downloadableSelection = items.length === 1 && items[0].kind === "file"; elements.scope.textContent = isMulti ? "Multi-selection" : "Single item"; - elements.target.textContent = isMulti ? `${items.length} selected items` : entry.name; elements.openButton.classList.toggle("hidden", isMulti); elements.openButton.disabled = !openableSingle; elements.editButton.classList.toggle("hidden", isMulti || items.length !== 1 || items[0].kind !== "file" || remoteSelection); @@ -819,13 +818,47 @@ function openContextMenu(pane, entry, event) { elements.propertiesButton.classList.remove("hidden"); elements.propertiesButton.disabled = items.length === 0; - const menuWidth = 220; - const menuHeight = 120; - const x = Math.min(event.clientX, window.innerWidth - menuWidth - 12); - const y = Math.min(event.clientY, window.innerHeight - menuHeight - 12); - elements.menu.style.left = `${Math.max(8, x)}px`; - elements.menu.style.top = `${Math.max(8, y)}px`; elements.menu.classList.remove("hidden"); + positionContextMenu(elements.menu, event.currentTarget, event); +} + +function positionContextMenu(menu, rowElement, event) { + if (!menu) { + return; + } + const paneElement = rowElement instanceof Element ? rowElement.closest(".pane") : null; + const paneRect = paneElement ? paneElement.getBoundingClientRect() : null; + const rowRect = rowElement instanceof Element ? rowElement.getBoundingClientRect() : null; + const menuRect = menu.getBoundingClientRect(); + const viewportPadding = 8; + const panePadding = 8; + + const minLeft = paneRect ? Math.max(viewportPadding, paneRect.left + panePadding) : viewportPadding; + const maxLeft = paneRect + ? Math.max(minLeft, Math.min(window.innerWidth - viewportPadding - menuRect.width, paneRect.right - panePadding - menuRect.width)) + : Math.max(minLeft, window.innerWidth - viewportPadding - menuRect.width); + const preferredLeft = rowRect ? rowRect.left + 12 : event.clientX; + const left = Math.max(minLeft, Math.min(maxLeft, preferredLeft)); + + const paneTop = paneRect ? paneRect.top + panePadding : viewportPadding; + const paneBottom = paneRect ? paneRect.bottom - panePadding : window.innerHeight - viewportPadding; + const rowTop = rowRect ? rowRect.top : event.clientY; + const rowBottom = rowRect ? rowRect.bottom : event.clientY; + const spaceBelow = paneBottom - rowBottom; + const spaceAbove = rowTop - paneTop; + + let top; + if (spaceBelow >= menuRect.height || spaceBelow >= spaceAbove) { + top = rowBottom; + } else if (spaceAbove >= menuRect.height) { + top = rowTop - menuRect.height; + } else { + top = Math.max(paneTop, Math.min(paneBottom - menuRect.height, rowBottom)); + } + top = Math.max(paneTop, Math.min(top, paneBottom - menuRect.height)); + + menu.style.left = `${left}px`; + menu.style.top = `${top}px`; } function applyContextMenuSelection() { diff --git a/webui/html/base.css b/webui/html/base.css index ba84c69..9f90867 100644 --- a/webui/html/base.css +++ b/webui/html/base.css @@ -907,7 +907,11 @@ button:disabled { .context-menu { position: fixed; - min-width: 220px; + display: inline-flex; + flex-direction: column; + align-items: stretch; + width: max-content; + max-width: min(196px, calc(100vw - 24px)); padding: 8px; border: 1px solid var(--color-border); border-radius: var(--radius-sm); @@ -924,24 +928,16 @@ button:disabled { color: var(--color-text-muted); } -.context-menu-target { - margin-top: 4px; - font-size: 12px; - color: var(--color-text-primary); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; -} - .context-menu-separator { height: 1px; - margin: 8px 0; + margin: 6px 0 8px; background: var(--color-border); } .context-menu button { - width: 100%; + width: auto; justify-content: flex-start; + white-space: nowrap; } #upload-modal .popup-card { diff --git a/webui/html/index.html b/webui/html/index.html index 4c8835d..36f2b20 100644 --- a/webui/html/index.html +++ b/webui/html/index.html @@ -161,7 +161,6 @@