diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 47955c0..eeb3874 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -183,7 +183,10 @@ async def _background_scan( await db.rollback() run = await db.get(ScanRun, run_id) if run and run.status == "running": - run.status = "failed" + # "error", the same word run_scan / run_device_scan write when + # they fail themselves — one condition, one name. Scan History + # filters and colours that one; "failed" showed up unlabelled. + run.status = "error" await db.commit() @@ -209,7 +212,10 @@ async def _background_device_scan( await db.rollback() run = await db.get(ScanRun, run_id) if run and run.status == "running": - run.status = "failed" + # "error", the same word run_scan / run_device_scan write when + # they fail themselves — one condition, one name. Scan History + # filters and colours that one; "failed" showed up unlabelled. + run.status = "error" await db.commit() diff --git a/backend/app/services/scanner.py b/backend/app/services/scanner.py index 94ad6ef..b437cdc 100644 --- a/backend/app/services/scanner.py +++ b/backend/app/services/scanner.py @@ -856,6 +856,11 @@ async def run_device_scan( "os": device.os, "open_ports": [], } + # A rescan re-observes a device through the source it already has; it + # is not a network discovery. Hardcoding "arp" here would tell a + # Proxmox guest, a rack mount or a hand-added host that the network + # scanner found it, and it would then answer that source filter. + source = device.discovery_source or "arp" # The full range goes out in slices so a stop request lands within one # slice instead of at the end, and so a spent budget keeps the ports @@ -912,7 +917,7 @@ async def run_device_scan( # The row is being rescanned on the user's request, so it is never # "hidden" from itself — the route rejects hidden devices upfront. outcome = await process_host( - db, host, hidden_ips=set(), deep_scan=deep_scan, discovery_source="arp" + db, host, hidden_ips=set(), deep_scan=deep_scan, discovery_source=source ) if outcome == "created": devices_found = 1 diff --git a/backend/tests/scan/test_scan_run.py b/backend/tests/scan/test_scan_run.py index 5fa9108..37793cb 100644 --- a/backend/tests/scan/test_scan_run.py +++ b/backend/tests/scan/test_scan_run.py @@ -12,9 +12,14 @@ from app.services.scanner import _cancelled_runs, request_cancel, run_scan @pytest.mark.asyncio -async def test_background_scan_marks_run_failed_on_exception(mem_db): - """If run_scan() raises, the ScanRun must transition running → failed and the - session rollback path must execute without a follow-on exception.""" +async def test_background_scan_marks_run_errored_on_exception(mem_db): + """If run_scan() raises, the ScanRun must transition running → error and the + session rollback path must execute without a follow-on exception. + + "error" and not "failed": one condition, one name — it is what run_scan + writes when it catches the failure itself, and the only one Scan History + filters and colours. + """ from app.api.routes.scan import _background_scan async with mem_db() as session: @@ -36,13 +41,13 @@ async def test_background_scan_marks_run_failed_on_exception(mem_db): async with mem_db() as session: refreshed = await session.get(ScanRun, run_id) assert refreshed is not None - assert refreshed.status == "failed" + assert refreshed.status == "error" @pytest.mark.asyncio async def test_background_scan_leaves_non_running_status_alone(mem_db): """If the run was already stopped/cancelled before run_scan failed, _background_scan - must NOT overwrite that terminal status with 'failed'.""" + must NOT overwrite that terminal status with 'error'.""" from app.api.routes.scan import _background_scan async with mem_db() as session: @@ -436,3 +441,30 @@ async def test_run_scan_updates_existing_pending_device(db_session: AsyncSession # Services and hostname should be updated assert device.hostname == "myhost.lan" assert any(s["port"] == 8096 for s in device.services) + + +@pytest.mark.asyncio +async def test_background_device_scan_marks_run_errored_on_exception(mem_db): + """Same word as the network scan — a device run that blows up reads alike.""" + from app.api.routes.scan import _background_device_scan + + async with mem_db() as session: + run = ScanRun(status="running", kind="device", ranges=["10.0.0.5/32"]) + session.add(run) + await session.commit() + run_id = run.id + + with ( + patch("app.api.routes.scan.AsyncSessionLocal", mem_db), + patch( + "app.api.routes.scan.run_device_scan", + new_callable=AsyncMock, + side_effect=RuntimeError("boom"), + ), + ): + await _background_device_scan(run_id, "d1") + + async with mem_db() as session: + refreshed = await session.get(ScanRun, run_id) + assert refreshed is not None + assert refreshed.status == "error" diff --git a/backend/tests/test_scanner.py b/backend/tests/test_scanner.py index e0a32c1..22e0ad2 100644 --- a/backend/tests/test_scanner.py +++ b/backend/tests/test_scanner.py @@ -1233,6 +1233,45 @@ async def test_scan_keeps_the_icon_the_user_picked_for_a_service(mem_db): assert ssh["category"] == "remote" +@pytest.mark.asyncio +async def test_run_device_scan_keeps_the_device_own_discovery_source(mem_db): + """A rescan re-observes a device; it does not discover it on the network. + + Tagging every rescanned device "arp" told a Proxmox guest or a hand-added + host that the network scanner found it, and it then answered that filter. + """ + from app.services.scanner import run_device_scan + + run_id = _make_run_id() + async with mem_db() as session: + session.add(ScanRun(id=run_id, status="running", kind="device", ranges=["192.168.1.9/32"])) + session.add(InventoryDevice( + id="d1", + ip="192.168.1.9", + status="approved", + services=[], + discovery_source="proxmox", + discovery_sources=["proxmox"], + )) + await session.commit() + + def _scanned(host_dict, port_spec, bounded=False): + host_dict["open_ports"] = [{"port": 8006, "protocol": "tcp", "banner": ""}] + return host_dict + + async with mem_db() as session: + with patch("app.services.scanner._nmap_scan_single", side_effect=_scanned), \ + patch("app.api.routes.status.broadcast_scan_update", new_callable=AsyncMock): + await run_device_scan("d1", session, run_id, ports="8006") + + async with mem_db() as session: + device = await session.get(InventoryDevice, "d1") + + assert device is not None + assert device.discovery_sources == ["proxmox"] + assert device.discovery_source == "proxmox" + + @pytest.mark.asyncio async def test_run_device_scan_marks_run_error_when_device_has_no_ip(mem_db): from app.services.scanner import run_device_scan diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 0d4b236..392c755 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -112,6 +112,8 @@ export type ScanConfigData = { ranges: string[] } & DeepScanConfig /** A row of `scan_runs` — what `/scan/runs` and the device rescan return. */ export interface ScanRunSummary { id: string + // 'failed' is legacy: runs recorded before the backend settled on 'error' + // for the same condition. Still read, never written. status: 'running' | 'done' | 'cancelled' | 'error' | 'failed' kind: string ranges: string[] diff --git a/frontend/src/components/modals/InventoryDeviceModal.tsx b/frontend/src/components/modals/InventoryDeviceModal.tsx index d316aed..dca522d 100644 --- a/frontend/src/components/modals/InventoryDeviceModal.tsx +++ b/frontend/src/components/modals/InventoryDeviceModal.tsx @@ -279,24 +279,35 @@ export function InventoryDeviceModal({ device, onClose, onApprove, onHide, onIgn onSavedRef.current = onSaved const editingRef = useRef(editing) editingRef.current = editing + // Read by the reset effect, which keys on the id alone — the object itself + // must not be a dependency. + const deviceRef = useRef(device) + deviceRef.current = device - // Reset the form whenever the modal is pointed at another device — the + // Reset the form whenever the modal is pointed at *another* device — the // component stays mounted across card clicks in the inventory grid. + // + // Keyed on the id, never on the object: the parent hands down a fresh + // `device` every time the row is refreshed from anywhere — a finishing deep + // rescan, a status push, a save — and resetting on those would drop an edit + // in progress along with the mode itself. A refreshed row is not a different + // device, and only a different device is a reason to throw the form away. + const deviceId = device?.id ?? null useEffect(() => { - if (!device) return - setForm(toForm(device)) - setProperties(device.properties ?? []) - setServices(device.services ?? []) + const d = deviceRef.current + if (!d) return + setForm(toForm(d)) + setProperties(d.properties ?? []) + setServices(d.services ?? []) setEditing(false) setSvcModal(null) setRescanRunId(null) setDeepScanOpen(false) - }, [device]) + }, [deviceId]) // Wait on a running deep rescan. A full 65535-port scan takes minutes, so the // run is polled rather than awaited — the user can close the modal, and the // scan keeps going and still shows under Scan History. - const deviceId = device?.id ?? null useEffect(() => { if (!rescanRunId || !deviceId) return let stopped = false diff --git a/frontend/src/components/modals/__tests__/InventoryDeviceModalRescan.test.tsx b/frontend/src/components/modals/__tests__/InventoryDeviceModalRescan.test.tsx index 0ad4cdc..2d7bc82 100644 --- a/frontend/src/components/modals/__tests__/InventoryDeviceModalRescan.test.tsx +++ b/frontend/src/components/modals/__tests__/InventoryDeviceModalRescan.test.tsx @@ -160,6 +160,49 @@ describe('InventoryDeviceModal — deep rescan', () => { expect(mockRun.mock.calls.length).toBe(calls) }) + it('keeps an edit in progress when the scan lands', async () => { + // The parent patches the row in place on onSaved (`setSelected(saved)`), + // handing down a new object for the same device. That must not throw away + // a form the user is still filling in — a deep scan runs for minutes. + vi.useFakeTimers({ shouldAdvanceTime: true }) + const onSaved = vi.fn() + const device = makeDevice() + const fresh = makeDevice({ + services: [{ port: 8096, protocol: 'tcp', service_name: 'Jellyfin' }], + }) + const { rerender } = render( + + ) + await startScan() + await waitFor(() => expect(screen.getByTestId('device-rescan-stop')).toBeInTheDocument()) + + fireEvent.click(screen.getByRole('button', { name: 'Edit' })) + fireEvent.change(screen.getByPlaceholderText('Display name'), { target: { value: 'Media box' } }) + + mockRun.mockResolvedValue({ data: { id: 'run-1', status: 'done', error: null } }) + mockPending.mockResolvedValue({ data: [fresh] }) + await act(async () => { await vi.advanceTimersByTimeAsync(3100) }) + + // The fresh row still reaches the canvas and the grid — the scan is not + // discarded just because a form is open. + await waitFor(() => expect(onSaved).toHaveBeenCalledWith(fresh)) + rerender() + + expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument() + expect(screen.getByDisplayValue('Media box')).toBeInTheDocument() + }) + + it('still resets the form when pointed at another device', async () => { + const { rerender } = render() + fireEvent.click(screen.getByRole('button', { name: 'Edit' })) + fireEvent.change(screen.getByPlaceholderText('Display name'), { target: { value: 'Media box' } }) + + rerender() + + expect(screen.queryByRole('button', { name: 'Save' })).toBeNull() + expect(screen.queryByDisplayValue('Media box')).toBeNull() + }) + it('says partial when the sweep ran out of budget', async () => { vi.useFakeTimers({ shouldAdvanceTime: true }) const { toast } = await import('sonner')