Address round-3 CodeRabbit findings

CHANGELOG.md

@@ -2,11 +2,6 @@
 
 ## [0.18.0] - 2026-05-04
 
-### Added
-
-- **LFTP hot-reload** — Connection-related settings (parallel connections, max total connections, socket buffer size, bandwidth limit) apply without a container restart. Settings UI distinguishes between options that take effect immediately and those that require restart (#433)
-- **Atomic config writes** — `Config.to_file()` now flushes to disk via temp file + `os.rename` so a process kill mid-write can't truncate the config (#433)
-
 ### Changed
 
 - **Image size reduced from 114 MB to 64 MB** — `RUN --mount=from=ghcr.io/astral-sh/uv` keeps `uv` out of the runtime image; build artifacts no longer persist (#437)
@@ -21,29 +16,30 @@
 - **Test count badges** added to the README (#451)
 - **Dependency updates** — Angular group (10 packages), typescript-eslint, jsdom, eslint, Docusaurus 3.10.0 → 3.10.1 across 5 packages (#458, #459, #460, #461, #463, #464, #465, #466)
 
+### Added
+
+- **LFTP hot-reload** — Connection-related settings (parallel connections, max total connections, socket buffer size, bandwidth limit) apply without a container restart. Settings UI distinguishes between options that take effect immediately and those that require restart (#433)
+- **Atomic config writes** — `Config.to_file()` now flushes to disk via temp file + `os.rename` so a process kill mid-write can't truncate the config (#433)
+- **Expanded unit and E2E test coverage:**
+  - 47 security middleware unit tests (#439)
+  - 36 controller core unit tests (#444)
+  - 28 FileOptions / Integrations / Option component tests (#448)
+  - 25 AutoQueueService and PathPairsService tests (#445)
+  - 20 HeaderComponent and VersionCheckService tests (#443)
+  - 18 ViewFileFilterService tests (#440)
+  - E2E for integrations CRUD (#441)
+  - E2E for File Actions & Error States (#438)
+  - E2E Settings coverage expansion (#442)
+  - Web App Job & Context Python tests (#446)
+  - Handler integration test expansion (#447)
+  - Python integration tests added to CI (#449)
+
 ### Fixed
 
 - **Hash Algorithm select test flake** — Test waited on the wrong config field; the algorithm select's disable gate is `validate.enabled`, not `validate.xfer_verify`. Test now sets the correct field and waits for the SSE-delivered model value before asserting (#455)
 - **StreamHandler leaks in test setUp methods** — Tests added a handler to the shared root logger but never removed it. Loggers are singletons, so by test N the logger had N handlers and each log line printed N times. Fixed via `self.addCleanup(logger.removeHandler, handler)` across 8 test files / 10 setUp methods (#450, #457)
 - **Multiprocessing resource leak in `test_active_scanner.py`** — `scanner.close()` now registered with `addCleanup` so resources release even if assertions raise
 
-### Tests
-
-This release brings a substantial expansion of unit and E2E coverage:
-
-- 47 security middleware unit tests (#439)
-- 36 controller core unit tests (#444)
-- 28 FileOptions / Integrations / Option component tests (#448)
-- 25 AutoQueueService and PathPairsService tests (#445)
-- 20 HeaderComponent and VersionCheckService tests (#443)
-- 18 ViewFileFilterService tests (#440)
-- E2E for integrations CRUD (#441)
-- E2E for File Actions & Error States (#438)
-- E2E Settings coverage expansion (#442)
-- Web App Job & Context Python tests (#446)
-- Handler integration test expansion (#447)
-- Python integration tests added to CI (#449)
-
 ## [0.17.0] - 2026-04-30
 
 ### Added

src/angular/src/app/services/settings/path-pairs.service.spec.ts

@@ -4,6 +4,7 @@ import { TestBed } from '@angular/core/testing';
 import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
 import { provideHttpClient } from '@angular/common/http';
 import { BehaviorSubject } from 'rxjs';
+import { take } from 'rxjs/operators';
 
 import { PathPairsService } from './path-pairs.service';
 import { ConnectedService } from '../utils/connected.service';
@@ -29,8 +30,11 @@ describe('PathPairsService', () => {
   let connectedSubject: BehaviorSubject<boolean>;
 
   function snapshot(): PathPair[] {
+    // take(1) auto-completes the observable after the first emission, so
+    // each snapshot() call doesn't accumulate a lingering subscription
+    // on service.pairs$.
     let result: PathPair[] = [];
-    service.pairs$.subscribe(p => result = p);
+    service.pairs$.pipe(take(1)).subscribe(p => result = p);
     return result;
   }
 

src/angular/src/app/services/utils/version-check.service.spec.ts

@@ -159,6 +159,7 @@ describe('VersionCheckService', () => {
 
     createService();
 
+    expect(notifications).toHaveLength(1);
     expect(notifications[0].text).toContain(url);
   });
 });

src/e2e-playwright/tests/integrations.spec.ts

@@ -251,12 +251,16 @@ test.describe("Integrations CRUD", () => {
     // Row should disappear
     await expect(row).not.toBeVisible({ timeout: 5000 });
 
-    // Verify via API
+    // Verify via API. Throw on a non-OK response so the poll keeps
+    // retrying — returning undefined here would falsely satisfy
+    // toBeUndefined() and mask a real API failure.
     await expect
       .poll(
         async () => {
           const res = await apiFetch("/server/integrations");
-          if (!res.ok) return undefined;
+          if (!res.ok) {
+            throw new Error(`GET /server/integrations failed: ${res.status} ${res.statusText}`);
+          }
           const instances = await res.json();
           return instances.find(
             (i: { name: string }) => i.name === "Delete Me"

src/e2e-playwright/tests/settings.spec.ts

@@ -181,8 +181,8 @@ test.describe("Settings Page", () => {
       const field = settings.getTextInput("Server Address");
       // fill() already clears before typing — calling clear() separately
       // races with Angular's signal-driven re-render and can reset the
-      // field before fill() runs.
-      await expect(field).toBeEnabled();
+      // field before fill() runs. Wait for SSE-delivered config first.
+      await expect(field).toBeEnabled({ timeout: 5000 });
       await field.fill("trigger-restart-notice-" + Date.now());
       await field.blur();
 
@@ -223,7 +223,7 @@ test.describe("Settings — Staging Directory", () => {
 
     try {
       const field = settings.getTextInput("Staging Path");
-      await expect(field).toBeEnabled();
+      await expect(field).toBeEnabled({ timeout: 5000 });
       const testValue = "/tmp/e2e-staging-" + Date.now();
       await field.fill(testValue);
       await field.blur();
@@ -405,7 +405,7 @@ test.describe("Settings — Connections", () => {
 
     try {
       const field = settings.getTextInput("Max Parallel Downloads");
-      await expect(field).toBeEnabled();
+      await expect(field).toBeEnabled({ timeout: 5000 });
       await field.fill("7");
       await field.blur();
 

src/python/tests/unittests/test_web/test_web_app_job.py

@@ -131,6 +131,10 @@ def failing_app(environ, start_response):
         def start_response(status, headers, *args):
             pass
 
-        with self.assertLogs("test_request_logging_error", level="DEBUG"):
+        with self.assertLogs("test_request_logging_error", level="DEBUG") as log_ctx:
             with self.assertRaises(RuntimeError):
                 list(middleware(environ, start_response))
+
+        # Mirror the happy-path assertion: the error branch should still log
+        # the method, path, and status that start_response saw before the raise.
+        self.assertTrue(any("POST" in o and "/fail" in o and "500" in o for o in log_ctx.output))