From e2dfc2652e3f2a08aaee42c03dfd96485719e70a Mon Sep 17 00:00:00 2001 From: Krzysztof Rudnicki Date: Mon, 3 Aug 2026 15:13:15 +0200 Subject: [PATCH] Recover instead of crashing when port 8730 is already taken Launching the app died with an unhandled SocketException whenever anything already held the wrapper port -- from `todo` and from `./run.sh` alike, since run.sh execs the same entry point. What was holding it was a wrapper from an earlier session that had outlived its window: /proc//exe read "/opt/todo/bin/todo_desktop (deleted)", i.e. install_arch.sh had replaced the binary underneath a process that kept serving anyway. That orphan is manufactured by the handoff branch in bin/todo_desktop.dart. When the launched browser exits within 5s the launcher assumes it handed the URL to an existing window and keeps serving rather than pulling the server out from under it -- right at that moment, but nothing ever reaps the server once that window closes. It then survives an upgrade and blocks every later launch. The port cannot simply move: the browser keys localStorage (the GitHub token) and IndexedDB (the notes) by origin, so a fallback port would present an empty, logged-out app. lib/desktop/port_guard.dart therefore decides who owns 8730 and acts: a wrapper with no window attached is stale, so it is terminated (SIGTERM, escalating to SIGKILL) and the port rebound with the current build; a wrapper that still has a window is joined, leaving exactly one server; anything else is named and refused. Reaping is what makes an upgrade self-healing, so install_arch.sh needs no restart step of its own -- one mechanism, not two. It fails closed at every step. `ss` hides the owning pid for another user's socket, and that is a real branch rather than an impossible one: it resolves to "refuse", never to a blind kill or a retry loop. Two things measurement contradicted, both of which had to change the design: Re-running the Chrome command does not focus the existing --app window, it opens a second one (verified on Thorium: the process returns in ~180ms and the window count goes 1 -> 2). So the join path opens a window instead of printing and exiting; both windows share the one server and one IndexedDB, and typing `todo` to get nothing visible would be the worse outcome. Chrome also rewrites its own argument vector into a single space-joined string, so /proc//cmdline for a browser contains no NUL separators at all. Exact token matching therefore never fired, and the first run of the live-window case reaped the server under an open window. Both matchers now tolerate either layout. The direction of that error matters and is commented as such: a false negative kills a server someone is using, a false positive merely opens a spare window. The decision logic lives in lib/ rather than in bin/, which is coverage:ignore-file -- otherwise the one path that has to work would be the only untested code in the repo. Host access sits behind an injected SystemProbe, and the ss parser is additionally exercised against genuine `ss` output by binding a real socket, since a fixture cannot catch a format change. Verified by running it, not only by tests: orphaned dev wrapper, orphaned AOT wrapper from the installed package, live window, a foreign owner (python http.server -- named, exit 1, and confirmed still alive afterwards), ./run.sh, and plain `todo`. 238 tests, 100% line coverage held. CLAUDE.md picks up the port guard, and drops two stale claims: the suite is 238 tests taking 1-2min, not 191 tests in ~5s -- that drift predates this change. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 15 +- bin/todo_desktop.dart | 105 +++++++++- lib/desktop/port_guard.dart | 315 ++++++++++++++++++++++++++++ test/port_guard_test.dart | 405 ++++++++++++++++++++++++++++++++++++ 4 files changed, 824 insertions(+), 16 deletions(-) create mode 100644 lib/desktop/port_guard.dart create mode 100644 test/port_guard_test.dart diff --git a/CLAUDE.md b/CLAUDE.md index a2f4296..8e9e83e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,8 +26,8 @@ and sync peer-to-peer through a GitHub repo used as dumb storage. ## Commands -- Run tests + coverage: `flutter test --coverage` (suite runs in ~5s; keep it - that fast — see Testing below). +- Run tests + coverage: `flutter test --coverage` (suite runs in 1–2min; keep it + quick — see Testing below). - Coverage summary: `lcov --list coverage/lcov.info`. - Static analysis: `flutter analyze` (must be clean before committing). - Format: `dart format lib/ test/`. @@ -36,7 +36,10 @@ and sync peer-to-peer through a GitHub repo used as dumb storage. - The wrapper serves on a **fixed** port 8730 with a **fixed** Chrome profile dir. Both must stay fixed: the GitHub token (`localStorage`) and the notes (IndexedDB) are keyed by origin and live in that profile, so changing either - silently logs you out and hides your notes. + silently logs you out and hides your notes. Because the port cannot move, + `lib/desktop/port_guard.dart` decides what to do when it is already taken: + reap a wrapper with no window attached (so an upgrade can't leave you on old + code), join one that still has a window, and refuse to touch anything else. - Release APK: `flutter build apk --release` (signs with the debug key; debug builds are janky — always measure smoothness on a release build). - Sync smoke test (hits real GitHub, needs a token): `dart run tool/sync_smoke.dart`. @@ -97,8 +100,10 @@ migrate → Settings → Import). ## Testing -- **The suite must stay fast (~5s) and fully green.** It is currently **191 - tests at 100% line coverage**. Don't regress either. +- **The suite must stay quick and fully green.** It is currently **238 tests at + 100% line coverage**, running in 1–2min on this machine. Don't regress either. + (The "~5s" figure this file used to quote had drifted well before the port + guard was added; measure before quoting a new one.) - Widget tests use `test/fake_note_repository.dart` — a `FakeNoteRepository` built on `StreamController`s, **not** a real DB. A real `sqlite_crdt` DB schedules timers that never drain under the widget tester's fake clock diff --git a/bin/todo_desktop.dart b/bin/todo_desktop.dart index 0c0b414..28f726a 100644 --- a/bin/todo_desktop.dart +++ b/bin/todo_desktop.dart @@ -1,10 +1,12 @@ // coverage:ignore-file // Entry point for the desktop wrapper: resolves real paths, starts the server, // and launches the browser. The serving logic it delegates to is covered by -// test/wrapper_server_test.dart. +// test/wrapper_server_test.dart, and the busy-port recovery it delegates to by +// test/port_guard_test.dart. Keep this file thin wiring for that reason. import 'dart:io'; import 'package:path/path.dart' as p; +import 'package:todo/desktop/port_guard.dart'; import 'package:todo/desktop/wrapper_server.dart'; /// Port must stay fixed: the browser keys `localStorage` (the GitHub token) and @@ -12,6 +14,10 @@ import 'package:todo/desktop/wrapper_server.dart'; /// app with no token and no notes. Kept in step with lib/sync/desktop_wrapper. const _port = 8730; +/// `errno` for EADDRINUSE. The wrapper is Linux-only (the desktop app is the +/// web build served locally), so the numeric value is stable here. +const _addressInUse = 98; + Future main(List args) async { final home = Platform.environment['HOME']; if (home == null) { @@ -23,7 +29,8 @@ Future main(List args) async { // installed layout is /opt/todo/bin/todo-desktop with the web assets one // level up at /opt/todo/web. --web-root overrides for development runs // straight out of the repo. - final webRoot = _argValue(args, '--web-root') ?? + final webRoot = + _argValue(args, '--web-root') ?? p.normalize(p.join(p.dirname(Platform.resolvedExecutable), '..', 'web')); if (!Directory(webRoot).existsSync()) { stderr.writeln('web assets not found at $webRoot'); @@ -36,10 +43,17 @@ Future main(List args) async { webRoot: webRoot, backlogPath: _argValue(args, '--backlog-path') ?? p.join(home, 'todo', 'BACKLOG.md'), - logPath: _argValue(args, '--log-path') ?? + logPath: + _argValue(args, '--log-path') ?? p.join(home, '.local', 'share', 'todo-desktop', 'todo_notes.json'), ); - await server.start(_port); + + if (!await _bind(server)) { + // Something already holds the port. It cannot simply move — the notes and + // the token are keyed by origin — so work out who owns it and either take + // it back, join it, or refuse. + if (!await _recoverPort(server, home, args)) return; + } stdout.writeln('todo desktop serving on http://localhost:$_port'); // A bare flag, not a valued option: requiring a dummy value meant passing a @@ -62,6 +76,77 @@ Future main(List args) async { } } +/// Binds [server] to the fixed port, reporting failure instead of throwing. +/// +/// Anything other than "address in use" is fatal and reported without a stack +/// trace: a Dart traceback tells the user nothing they can act on. +Future _bind(WrapperServer server) async { + try { + await server.start(_port); + return true; + } on SocketException catch (e) { + if (e.osError?.errorCode == _addressInUse) return false; + stderr.writeln('could not serve on port $_port: ${e.osError?.message}'); + exit(1); + } +} + +/// Resolves a busy port and, where possible, gets [server] listening. +/// +/// Returns true once the server is bound. Returns false when this process has +/// deliberately not taken the port — it joined a running instance instead — in +/// which case the caller must not touch [server]: it owns no socket, and +/// falling through to the browser-lifetime logic would leave a second orphan. +/// Exits the process when the port cannot safely be had. +Future _recoverPort( + WrapperServer server, + String home, + List args, +) async { + final guard = PortGuard(profileDir: _profileDir(home)); + switch (await guard.resolve(_port)) { + case AbortWithOwner(:final description): + stderr.writeln('port $_port is already in use by $description.'); + stderr.writeln( + 'todo needs this exact port — its notes and GitHub token are stored ' + 'per origin. Free the port, then run todo again.', + ); + exit(1); + case AttachToExisting(:final pid): + // Re-running the browser command opens a second window rather than + // raising the first (measured on Thorium: the process returns in ~180ms + // and window count goes 1 -> 2). A duplicate window is the right + // trade-off anyway: both share this one server and one IndexedDB, and + // typing `todo` to get nothing visible would be worse. + final opening = !args.contains('--no-browser'); + stdout.writeln( + 'todo is already running (pid $pid)' + '${opening ? '; opening a window.' : '.'}', + ); + if (opening) await _launchBrowser(home); + return false; + case ReapThenBind(:final pid): + stdout.writeln('clearing a stale todo wrapper (pid $pid) on port $_port'); + if (!await guard.reap(pid, _port)) { + stderr.writeln('could not free port $_port from pid $pid.'); + exit(1); + } + case BindNow(): + break; + } + if (await _bind(server)) return true; + stderr.writeln('port $_port was taken again while todo was starting.'); + exit(1); +} + +/// Browser profile directory for the desktop app. +/// +/// Must stay stable: the GitHub token (localStorage) and the notes (IndexedDB) +/// live inside it, so a changing path silently logs the user out and hides +/// their notes. It doubles as the marker for "a window is still attached". +String _profileDir(String home) => + p.join(home, '.local', 'share', 'todo-desktop', 'profile'); + /// Launches the app in a Chrome-family browser with a **stable** profile /// directory, since the token and notes live in that profile. /// Returns true when the browser ran long enough to have owned the session. @@ -82,17 +167,15 @@ Future _launchBrowser(String home) async { orElse: () => '', ); if (browser.isEmpty) { - stderr.writeln('No Chrome-family browser found; open ' - 'http://localhost:$_port manually.'); + stderr.writeln( + 'No Chrome-family browser found; open ' + 'http://localhost:$_port manually.', + ); return false; } - // The profile directory must stay stable: the GitHub token (localStorage) - // and the notes (IndexedDB) live inside it, so a changing path silently - // logs the user out and hides their notes. - final profile = p.join(home, '.local', 'share', 'todo-desktop', 'profile'); final process = await Process.start(browser, [ '--app=http://localhost:$_port', - '--user-data-dir=$profile', + '--user-data-dir=${_profileDir(home)}', // Sets WM_CLASS, which the .desktop entry matches on via StartupWMClass. // Without it the window inherits the browser's class and the taskbar shows // a browser icon instead of todo's. diff --git a/lib/desktop/port_guard.dart b/lib/desktop/port_guard.dart new file mode 100644 index 0000000..05ec8c0 --- /dev/null +++ b/lib/desktop/port_guard.dart @@ -0,0 +1,315 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as p; + +/// NUL separates the arguments inside `/proc//cmdline`. +final String _argumentSeparator = String.fromCharCode(0); + +/// What the launcher should do when the fixed wrapper port is already bound. +/// +/// The port cannot move — the browser keys `localStorage` (the GitHub token) +/// and IndexedDB (the notes) by origin, so falling back to a free port would +/// silently present an empty, logged-out app. The only options are therefore to +/// take the port back, share the server that already owns it, or refuse. +sealed class PortAction { + /// Allows subclasses to be const. + const PortAction(); +} + +/// The port came free between the failed bind and the probe: just bind again. +final class BindNow extends PortAction { + /// Creates the retry action. + const BindNow(); +} + +/// A todo wrapper is serving and a browser window is still attached to it. +/// +/// Reaping here would pull the server out from under a window the user is +/// looking at, so the launcher joins the running instance instead. +final class AttachToExisting extends PortAction { + /// Creates the attach action for the wrapper running as [pid]. + const AttachToExisting(this.pid); + + /// Process id of the wrapper already serving the port. + final int pid; +} + +/// A todo wrapper is serving with no window attached — a leftover from an +/// earlier session, safe to terminate so the current build can take over. +final class ReapThenBind extends PortAction { + /// Creates the reap action for the stale wrapper running as [pid]. + const ReapThenBind(this.pid); + + /// Process id of the stale wrapper holding the port. + final int pid; +} + +/// The port is held by something this launcher must not kill. +final class AbortWithOwner extends PortAction { + /// Creates the abort action, describing the owner for the error message. + const AbortWithOwner(this.description); + + /// Human-readable description of the owning process. + final String description; +} + +/// Identifying details of a running process, as read from `/proc`. +class ProcessDescription { + /// Creates a description from an executable path and an argument vector. + const ProcessDescription({required this.exeTarget, required this.cmdline}); + + /// Target of `/proc//exe`; empty when it could not be read. + /// + /// Carries a ` (deleted)` suffix when the binary was replaced underneath the + /// running process — exactly what an upgrade leaves behind. + final String exeTarget; + + /// Argument vector, already split on the NUL separators `/proc` uses. + final List cmdline; +} + +/// Read-only view of the host that [PortGuard] reasons about. +/// +/// Injectable so the decision logic can be tested without live processes. +abstract interface class SystemProbe { + /// Process id listening on [port], or null when it cannot be determined. + Future listenerPid(int port); + + /// Describes [pid], or null when the process is gone. + Future describe(int pid); + + /// Argument vectors of every process readable by this user. + Future>> processCmdlines(); + + /// Whether [port] currently refuses a loopback bind. + Future portInUse(int port); + + /// Sends [signal] to [pid]; returns whether the signal was delivered. + bool signal(int pid, ProcessSignal signal); +} + +/// Extracts the process id listening on [port] from `ss -tlnp` output. +/// +/// Returns null when no line matches or the owning pid is hidden, which `ss` +/// does for sockets belonging to other users. +int? parseListenerPid(String ssOutput, int port) { + for (final line in const LineSplitter().convert(ssOutput)) { + final fields = line + .split(RegExp(r'\s+')) + .where((f) => f.isNotEmpty) + .toList(); + if (fields.length < 4) continue; + // Field 3 is `Local Address:Port`. Matching on the port alone (rather than + // the address too) keeps IPv6 and wildcard listeners in scope; a wrong + // guess is harmless because the caller still has to prove the process is + // a wrapper of this app before it may kill anything. + if (!fields[3].endsWith(':$port')) continue; + final pid = RegExp(r'pid=(\d+)').firstMatch(line)?.group(1); + if (pid != null) return int.parse(pid); + } + return null; +} + +/// Whether the process described by [exeTarget] and [cmdline] is a wrapper. +/// +/// Matches both the installed AOT binary (`/opt/todo/bin/todo_desktop`, with or +/// without the ` (deleted)` suffix an upgrade leaves) and a development run out +/// of the repo (`dart run bin/todo_desktop.dart`, as `run.sh` starts it). +bool isTodoWrapper(String exeTarget, List cmdline) { + final exe = p.basename(exeTarget.replaceAll(' (deleted)', '')); + if (exe == 'todo_desktop') return true; + // A plain substring match is deliberate. It is only ever asked about the + // process holding the wrapper port, which no unrelated program does, so the + // looser test cannot reach a bystander — whereas a stricter one would refuse + // to reap a wrapper whose argument vector does not split the way we expect, + // leaving the user unable to start the app at all. + return cmdline.join(' ').contains('todo_desktop'); +} + +/// Whether any process in [cmdlines] is a browser holding [profileDir] open. +/// +/// A live window is the one thing that makes reaping the port owner harmful, so +/// this is the discriminator between "stale leftover" and "in use". +/// +/// Chrome rewrites its own argument vector into one space-joined string, so +/// `/proc//cmdline` for a browser has no NUL separators at all and the +/// flag has to be found inside a token rather than as one. Getting this wrong +/// is asymmetric: a false negative kills the server under a window the user is +/// using, while a false positive merely opens an extra window, so this errs +/// towards detecting attachment. +bool hasWindowAttached(Iterable> cmdlines, String profileDir) { + final flag = '--user-data-dir=$profileDir'; + return cmdlines.any((tokens) { + final joined = tokens.join(' '); + final start = joined.indexOf(flag); + if (start == -1) return false; + // Require a boundary so a sibling profile (`…/profile2`) cannot match. + final end = start + flag.length; + return end == joined.length || joined[end] == ' '; + }); +} + +/// Decides how the launcher should react to a busy wrapper port, and carries +/// out the termination when the owner turns out to be stale. +/// +/// Fails closed throughout: anything it cannot positively identify as a todo +/// wrapper is reported rather than killed. +class PortGuard { + /// Creates a guard for the browser profile at [profileDir]. + PortGuard({required this.profileDir, SystemProbe? probe}) + : probe = probe ?? const LinuxSystemProbe(); + + /// Browser profile directory whose presence in a command line means a window + /// is still attached to the running wrapper. + final String profileDir; + + /// Host probe used to identify the port owner. + final SystemProbe probe; + + /// Determines what to do about [port] after a bind failed. + Future resolve(int port) async { + final pid = await probe.listenerPid(port); + if (pid == null) { + // Either the owner released the port, or `ss` hid it because it belongs + // to another user. Only the former is safe to retry. + return await probe.portInUse(port) + ? const AbortWithOwner( + 'an unidentified process (it may belong to another user)', + ) + : const BindNow(); + } + final description = await probe.describe(pid); + // The process exited while we were looking at it. + if (description == null) return const BindNow(); + if (!isTodoWrapper(description.exeTarget, description.cmdline)) { + final exe = description.exeTarget.isEmpty + ? description.cmdline.firstOrNull ?? 'unknown' + : description.exeTarget; + return AbortWithOwner('pid $pid ($exe)'); + } + return hasWindowAttached(await probe.processCmdlines(), profileDir) + ? AttachToExisting(pid) + : ReapThenBind(pid); + } + + /// Terminates [pid] and waits for [port] to come free. + /// + /// Escalates from `SIGTERM` to `SIGKILL` rather than waiting indefinitely. + /// The wrapper needs no graceful shutdown: it holds no unflushed state, since + /// every `/backup/*` write completes within its request. + /// + /// Returns whether [port] was actually released. + Future reap( + int pid, + int port, { + Duration termWait = const Duration(seconds: 3), + Duration killWait = const Duration(seconds: 2), + Duration pollInterval = const Duration(milliseconds: 50), + }) async { + probe.signal(pid, ProcessSignal.sigterm); + if (await _awaitFreePort(port, termWait, pollInterval)) return true; + probe.signal(pid, ProcessSignal.sigkill); + return _awaitFreePort(port, killWait, pollInterval); + } + + Future _awaitFreePort( + int port, + Duration limit, + Duration pollInterval, + ) async { + final deadline = DateTime.now().add(limit); + do { + if (!await probe.portInUse(port)) return true; + await Future.delayed(pollInterval); + } while (DateTime.now().isBefore(deadline)); + return !await probe.portInUse(port); + } +} + +/// [SystemProbe] backed by `ss` and `/proc`. +/// +/// The desktop wrapper only ever runs on Linux — the desktop app is the web +/// build served locally — so reading `/proc` directly is safe here. +class LinuxSystemProbe implements SystemProbe { + /// Creates the default host probe. + /// + /// [ssExecutable] exists so a test can point at a missing binary and exercise + /// the "no `ss` on PATH" path. + const LinuxSystemProbe({this.ssExecutable = 'ss'}); + + /// Name of the socket-listing tool used to find the port owner. + final String ssExecutable; + + @override + Future listenerPid(int port) async { + try { + final result = await Process.run(ssExecutable, ['-tlnp']); + final out = result.stdout; + return out is String ? parseListenerPid(out, port) : null; + } on ProcessException { + // No `ss` on PATH: report "unknown" and let the caller fail closed. + return null; + } + } + + @override + Future describe(int pid) async { + final cmdline = readCmdline(pid); + if (cmdline == null) return null; + String exe; + try { + exe = Link('/proc/$pid/exe').targetSync(); + } on FileSystemException { + // Unreadable for another user's process; the cmdline still identifies it. + exe = ''; + } + return ProcessDescription(exeTarget: exe, cmdline: cmdline); + } + + @override + Future>> processCmdlines() async { + final cmdlines = >[]; + for (final entry in Directory('/proc').listSync()) { + final pid = int.tryParse(p.basename(entry.path)); + if (pid == null) continue; + final cmdline = readCmdline(pid); + if (cmdline != null) cmdlines.add(cmdline); + } + return cmdlines; + } + + @override + Future portInUse(int port) async { + try { + final socket = await ServerSocket.bind( + InternetAddress.loopbackIPv4, + port, + ); + await socket.close(); + return false; + } on SocketException { + return true; + } + } + + @override + bool signal(int pid, ProcessSignal signal) => Process.killPid(pid, signal); + + /// Reads `/proc//cmdline` as an argument vector. + /// + /// Returns null when the process is gone. Kernel threads yield an empty list, + /// and the trailing separator is dropped so exact-token matching holds. + static List? readCmdline(int pid) { + try { + return File('/proc/$pid/cmdline') + .readAsStringSync() + .split(_argumentSeparator) + .where((token) => token.isNotEmpty) + .toList(); + } on FileSystemException { + return null; + } + } +} diff --git a/test/port_guard_test.dart b/test/port_guard_test.dart new file mode 100644 index 0000000..26c2850 --- /dev/null +++ b/test/port_guard_test.dart @@ -0,0 +1,405 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:todo/desktop/port_guard.dart'; + +/// Real `ss -tlnp` output, including the header row and the run-together +/// `Peer Address:PortProcess` column that iproute2 actually prints. +const _ssOutput = ''' +State Recv-Q Send-Q Local Address:Port Peer Address:PortProcess +LISTEN 0 4096 0.0.0.0:5355 0.0.0.0:* +LISTEN 0 128 127.0.0.1:8730 0.0.0.0:* users:(("dart:todo_deskt",pid=61687,fd=7)) +LISTEN 0 511 127.0.0.1:9000 0.0.0.0:* users:(("node",pid=1234,fd=20)) +'''; + +/// A [SystemProbe] whose every answer is scripted by the test. +class FakeProbe implements SystemProbe { + FakeProbe({ + this.pid, + this.description, + this.cmdlines = const [], + this.inUse = const [true], + this.freeAfter, + }); + + int? pid; + ProcessDescription? description; + List> cmdlines; + + /// Answers for successive [portInUse] calls; the last one repeats. + List inUse; + + /// When set, the port frees only once this signal has been delivered. + /// + /// Keyed on the signal rather than on elapsed time so the escalation test + /// cannot flake on a slow machine. + ProcessSignal? freeAfter; + + final List signalsSent = []; + int portInUseCalls = 0; + + @override + Future listenerPid(int port) async => pid; + + @override + Future describe(int pid) async => description; + + @override + Future>> processCmdlines() async => cmdlines; + + @override + Future portInUse(int port) async { + portInUseCalls++; + if (freeAfter != null) return !signalsSent.contains(freeAfter); + return inUse[(portInUseCalls - 1).clamp(0, inUse.length - 1)]; + } + + @override + bool signal(int pid, ProcessSignal signal) { + signalsSent.add(signal); + return true; + } +} + +const _profile = '/home/kuchy/.local/share/todo-desktop/profile'; + +/// Fast timings so the reap tests do not slow the suite down. +Future _reap(PortGuard guard, int pid, int port) => guard.reap( + pid, + port, + termWait: const Duration(milliseconds: 60), + killWait: const Duration(milliseconds: 60), + pollInterval: const Duration(milliseconds: 5), +); + +void main() { + group('parseListenerPid', () { + test('finds the pid holding the requested port', () { + expect(parseListenerPid(_ssOutput, 8730), 61687); + expect(parseListenerPid(_ssOutput, 9000), 1234); + }); + + test('returns null when no line listens on the port', () { + expect(parseListenerPid(_ssOutput, 7777), isNull); + }); + + test('returns null when ss hides the owning pid', () { + // ss omits the users:(...) column for another user's socket. + const hidden = 'LISTEN 0 128 127.0.0.1:8730 0.0.0.0:*'; + expect(parseListenerPid(hidden, 8730), isNull); + }); + + test('ignores short and empty lines', () { + expect(parseListenerPid('\nLISTEN 0 128\n', 8730), isNull); + }); + + test('does not match a port that is only a suffix', () { + const other = + 'LISTEN 0 128 127.0.0.1:18730 0.0.0.0:* ' + 'users:(("x",pid=5,fd=7))'; + expect(parseListenerPid(other, 8730), isNull); + }); + }); + + group('isTodoWrapper', () { + test('accepts the installed binary', () { + expect(isTodoWrapper('/opt/todo/bin/todo_desktop', []), isTrue); + }); + + test('accepts a binary replaced by an upgrade', () { + expect( + isTodoWrapper('/opt/todo/bin/todo_desktop (deleted)', []), + isTrue, + ); + }); + + test('accepts a development run out of the repo', () { + expect( + isTodoWrapper('/usr/lib/dart/bin/dart', [ + 'dart', + 'run', + 'bin/todo_desktop.dart', + '--web-root', + 'build/web', + ]), + isTrue, + ); + }); + + test('rejects an unrelated process', () { + expect( + isTodoWrapper('/usr/bin/python3.14', [ + 'python3', + '-m', + 'http.server', + '8730', + ]), + isFalse, + ); + }); + }); + + group('hasWindowAttached', () { + // Chrome rewrites argv into one space-joined string, so this is the shape + // that actually appears in /proc for a browser window. + const collapsed = [ + '/opt/thorium-browser/thorium --app=http://localhost:8730 ' + '--user-data-dir=$_profile --class=todo --no-first-run', + ]; + + test('matches a browser whose argv collapsed into one token', () { + expect(hasWindowAttached([collapsed], _profile), isTrue); + }); + + test('matches a normally separated argument vector', () { + expect( + hasWindowAttached( + [ + ['thorium', '--user-data-dir=$_profile', '--class=todo'], + ], + _profile, + ), + isTrue, + ); + }); + + test('matches when the flag is the final argument', () { + expect( + hasWindowAttached( + [ + ['thorium', '--user-data-dir=$_profile'], + ], + _profile, + ), + isTrue, + ); + }); + + test('does not match a sibling profile directory', () { + expect( + hasWindowAttached( + [ + ['thorium', '--user-data-dir=${_profile}2'], + ], + _profile, + ), + isFalse, + ); + }); + + test('does not match when no process mentions the profile', () { + expect( + hasWindowAttached( + [ + ['bash', '-c', 'sleep 1'], + ], + _profile, + ), + isFalse, + ); + }); + }); + + group('PortGuard.resolve', () { + test('retries the bind when the port came free', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe(inUse: [false]), + ); + expect(await guard.resolve(8730), isA()); + }); + + test('refuses when the owner cannot be identified', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe(inUse: [true]), + ); + final action = await guard.resolve(8730); + expect(action, isA()); + expect( + (action as AbortWithOwner).description, + contains('unidentified'), + ); + }); + + test('retries when the owner exited mid-probe', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe(pid: 42), + ); + expect(await guard.resolve(8730), isA()); + }); + + test('refuses to touch a process that is not a wrapper', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe( + pid: 42, + description: const ProcessDescription( + exeTarget: '/usr/bin/python3.14', + cmdline: ['python3', '-m', 'http.server'], + ), + ), + ); + final action = await guard.resolve(8730); + expect(action, isA()); + expect((action as AbortWithOwner).description, contains('42')); + expect(action.description, contains('python3.14')); + }); + + test('names the command when the executable is unreadable', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe( + pid: 42, + description: const ProcessDescription( + exeTarget: '', + cmdline: ['some-daemon'], + ), + ), + ); + final action = await guard.resolve(8730); + expect((action as AbortWithOwner).description, contains('some-daemon')); + }); + + test('reports unknown when there is no command either', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe( + pid: 42, + description: const ProcessDescription(exeTarget: '', cmdline: []), + ), + ); + final action = await guard.resolve(8730); + expect((action as AbortWithOwner).description, contains('unknown')); + }); + + test('attaches when a browser window is still open', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe( + pid: 42, + description: const ProcessDescription( + exeTarget: '/opt/todo/bin/todo_desktop', + cmdline: ['/opt/todo/bin/todo_desktop'], + ), + cmdlines: const [ + ['thorium', '--user-data-dir=$_profile'], + ], + ), + ); + final action = await guard.resolve(8730); + expect(action, isA()); + expect((action as AttachToExisting).pid, 42); + }); + + test('reaps a wrapper with no window attached', () async { + final guard = PortGuard( + profileDir: _profile, + probe: FakeProbe( + pid: 42, + description: const ProcessDescription( + exeTarget: '/opt/todo/bin/todo_desktop', + cmdline: ['/opt/todo/bin/todo_desktop'], + ), + ), + ); + final action = await guard.resolve(8730); + expect(action, isA()); + expect((action as ReapThenBind).pid, 42); + }); + + test('defaults to the real host probe', () { + expect(PortGuard(profileDir: _profile).probe, isA()); + }); + }); + + group('PortGuard.reap', () { + test('stops at SIGTERM when the port comes free', () async { + final probe = FakeProbe(freeAfter: ProcessSignal.sigterm); + final guard = PortGuard(profileDir: _profile, probe: probe); + expect(await _reap(guard, 42, 8730), isTrue); + expect(probe.signalsSent, [ProcessSignal.sigterm]); + }); + + test('escalates to SIGKILL when SIGTERM is ignored', () async { + // Busy for the whole SIGTERM wait, free once SIGKILL lands. + final probe = FakeProbe(freeAfter: ProcessSignal.sigkill); + final guard = PortGuard(profileDir: _profile, probe: probe); + expect(await _reap(guard, 42, 8730), isTrue); + expect(probe.signalsSent, [ + ProcessSignal.sigterm, + ProcessSignal.sigkill, + ]); + }); + + test('reports failure when the port never comes free', () async { + final probe = FakeProbe(inUse: [true]); + final guard = PortGuard(profileDir: _profile, probe: probe); + expect(await _reap(guard, 42, 8730), isFalse); + expect(probe.signalsSent, [ + ProcessSignal.sigterm, + ProcessSignal.sigkill, + ]); + }); + }); + + group('LinuxSystemProbe against the real host', () { + test('portInUse tracks a real socket', () async { + const probe = LinuxSystemProbe(); + final socket = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0); + addTearDown(() async => socket.close()); + expect(await probe.portInUse(socket.port), isTrue); + await socket.close(); + expect(await probe.portInUse(socket.port), isFalse); + }); + + test('listenerPid finds this process holding a real port', () async { + const probe = LinuxSystemProbe(); + final socket = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0); + addTearDown(() async => socket.close()); + // Proves the parser works against genuine `ss` output, not a fixture. + expect(await probe.listenerPid(socket.port), pid); + }); + + test('listenerPid returns null when ss is unavailable', () async { + const probe = LinuxSystemProbe(ssExecutable: 'ss-does-not-exist'); + expect(await probe.listenerPid(8730), isNull); + }); + + test('describe reads this process', () async { + const probe = LinuxSystemProbe(); + final description = await probe.describe(pid); + expect(description, isNotNull); + expect(description!.cmdline, isNotEmpty); + expect(description.exeTarget, isNotEmpty); + }); + + test('describe tolerates an unreadable executable link', () async { + // pid 1 belongs to root: its cmdline is world-readable but /proc/1/exe + // is not, which is the branch under test. + const probe = LinuxSystemProbe(); + final description = await probe.describe(1); + expect(description, isNotNull); + expect(description!.exeTarget, isEmpty); + }); + + test('describe returns null for a process that does not exist', () async { + const probe = LinuxSystemProbe(); + expect(await probe.describe(0x7FFFFFFF), isNull); + }); + + test('processCmdlines lists real processes', () async { + const probe = LinuxSystemProbe(); + final cmdlines = await probe.processCmdlines(); + expect(cmdlines, isNotEmpty); + }); + + test('signal terminates a real child process', () async { + const probe = LinuxSystemProbe(); + final child = await Process.start('sleep', ['30']); + expect(probe.signal(child.pid, ProcessSignal.sigterm), isTrue); + expect(await child.exitCode, isNot(0)); + }); + }); +}