fix(profiles): finish the fourth review round (SPEC-50)

Completes the CodeRabbit/Macroscope round begun in 288b7b2.

- desktop_controller: serialize start/stop/restart. They share the daemon's PID
  file and control socket (and restart is a stop+start), so overlapping requests
  could drop a fresh PID, launch a second daemon, or stop one another action had
  just started. Each action now chains after the previous, with a regression test
  asserting they never overlap.
- profile_lifecycle: keep the pid-file read synchronous. Making it async added
  real filesystem microtasks that a widget test's pumpAndSettle can never settle,
  which hung the delete tests. Only `processAlive` (polled ~100x per stop) needed
  to be async, and it stays async.
- test doubles: override `withLock` in the two "writes nothing" FileSystemAdapter
  fakes. They inherited the real implementation, which created `<path>.lock` on
  the real disk under a non-existent /Users/test — the actual cause of the hang.
- profiles_section_test: replace `await Future.delayed(...)` with
  `tester.pump(...)`; inside testWidgets the clock only advances when pumping, so
  the bare delay deadlocked.
- profile_registry: keep the already-persisted profile's port when reconciling a
  post-merge collision (its daemon may be running on it) and reassign the
  newcomer instead; test asserts which side moves.
- profile_runtime_test: assert dispose() disposes profilesController.
- server_devices_section_test: reuse `_pump` via an optional NavigatorObserver.
- docs/mockup: record the 64-code-point cap on the pair-URL `n` param, fix the
  delta cross-references (18→19, 17→18), use `pnpm typecheck` as the documented
  server check, and correct two stale "explicitly not changed" claims.

Author: leduckhc

app/lib/desktop/daemon/profile_lifecycle.dart

@@ -61,13 +61,13 @@ class ProfileLifecycle {
     bool Function(String path)? socketExists,
     Future<bool> Function(ServerProfile profile)? statusProbe,
     Future<void> Function(Duration duration)? sleep,
-    Future<int?> Function(ServerProfile profile)? readPid,
+    int? Function(ServerProfile profile)? readPid,
     Future<bool> Function(int pid)? processAlive,
   }) : run = run ?? _defaultRun,
        _socketExists = socketExists ?? _fileExists,
        _statusProbe = statusProbe ?? _connectProbe,
        _sleep = sleep ?? Future<void>.delayed,
-       _readPid = readPid ?? _readPidFileAsync,
+       _readPid = readPid ?? _readPidFile,
        _processAlive = processAlive ?? _posixProcessAlive;
 
   /// Locates the `makit` executable.
@@ -79,7 +79,7 @@ class ProfileLifecycle {
   final bool Function(String path) _socketExists;
   final Future<bool> Function(ServerProfile profile) _statusProbe;
   final Future<void> Function(Duration duration) _sleep;
-  final Future<int?> Function(ServerProfile profile) _readPid;
+  final int? Function(ServerProfile profile) _readPid;
   final Future<bool> Function(int pid) _processAlive;
 
   /// Runs `MAKIT_HOME=<profile.home> makit start`.
@@ -123,7 +123,7 @@ class ProfileLifecycle {
     ServerProfile profile, {
     Duration timeout = _kDefaultStopTimeout,
   }) async {
-    final pid = await _readPid(profile);
+    final pid = _readPid(profile);
     final stopResult = await stop(profile);
     if (stopResult.outcome == DaemonActionOutcome.failed ||
         stopResult.outcome == DaemonActionOutcome.cliNotFound) {
@@ -188,12 +188,16 @@ class ProfileLifecycle {
 
   /// Reads the daemon pid from `$MAKIT_HOME/makit.pid`, or `null` when the file
   /// is absent or unparseable. Read before `makit stop`, which deletes it.
-  static Future<int?> _readPidFileAsync(ServerProfile profile) async {
+  ///
+  /// Synchronous on purpose: it is one small read, once per stop — unlike
+  /// [_posixProcessAlive], which is polled up to ~100 times and therefore must
+  /// be async. Making this async introduced real filesystem microtasks that a
+  /// widget test's `pumpAndSettle` could never settle, hanging the delete tests.
+  static int? _readPidFile(ServerProfile profile) {
     try {
       final file = File(profile.pidFilePath);
-      if (!await file.exists()) return null;
-      final raw = await file.readAsString();
-      return int.tryParse(raw.trim());
+      if (!file.existsSync()) return null;
+      return int.tryParse(file.readAsStringSync().trim());
     } on FileSystemException {
       return null;
     }

app/lib/desktop/desktop_controller.dart

@@ -121,26 +121,50 @@ class DesktopController extends ChangeNotifier {
     Future<DaemonActionResult> Function() action, {
     DaemonState? transient,
   }) async {
-    if (transient != null) {
-      // Counts as newer than any refresh already in flight: an action's
-      // `starting`/`stopping` state has to survive until the action's own
-      // refresh replaces it, or a poll that left before the button was pressed
-      // repaints it as stopped mid-start.
-      ++_refreshGeneration;
-      _summary = DaemonSummary(
-        state: transient,
-        pid: _summary.pid,
-        pairedDevices: _summary.pairedDevices,
-        runningSessions: _summary.runningSessions,
-      );
-      notifyListeners();
+    // Serialize lifecycle actions. `start`/`stop`/`restart` share the daemon's
+    // PID file and control socket (and `restart` is a `stop` then `start`), so
+    // overlapping requests — from mashing Start/Stop/Restart or a reachability
+    // change racing a manual action — can remove a freshly written PID file,
+    // launch a second daemon, or stop one another action just started. Chaining
+    // each action after the previous one makes them strictly sequential.
+    final prior = _actionTail;
+    final done = Completer<void>();
+    _actionTail = done.future;
+    if (prior != null) {
+      try {
+        await prior;
+      } catch (_) {
+        // A prior action's failure must not cancel the ones queued behind it.
+      }
+    }
+    try {
+      if (transient != null) {
+        // Counts as newer than any refresh already in flight: an action's
+        // `starting`/`stopping` state has to survive until the action's own
+        // refresh replaces it, or a poll that left before the button was pressed
+        // repaints it as stopped mid-start.
+        ++_refreshGeneration;
+        _summary = DaemonSummary(
+          state: transient,
+          pid: _summary.pid,
+          pairedDevices: _summary.pairedDevices,
+          runningSessions: _summary.runningSessions,
+        );
+        notifyListeners();
+      }
+      final result = await action();
+      _cliMissing = result.outcome == DaemonActionOutcome.cliNotFound;
+      await refresh();
+      return result;
+    } finally {
+      if (identical(_actionTail, done.future)) _actionTail = null;
+      done.complete();
     }
-    final result = await action();
-    _cliMissing = result.outcome == DaemonActionOutcome.cliNotFound;
-    await refresh();
-    return result;
   }
 
+  /// Tail of the serialized lifecycle-action chain, or null when idle.
+  Future<void>? _actionTail;
+
   /// Starts periodic polling: refreshes immediately, then every [interval]
   /// while the window is visible, dropping to [hiddenInterval] while it is not.
   /// Cancels any previous poll. Owned here so the app can stop it cleanly on

app/lib/desktop/desktop_controller_test.dart

@@ -206,6 +206,53 @@ void main() {
       expect(calls.single, ['/x/makit', 'stop']);
     });
 
+    test('lifecycle actions are serialized, never overlapping', () async {
+      // start/stop/restart share the PID file and control socket, so overlapping
+      // them can drop a fresh PID or launch a second daemon. They must run
+      // strictly one at a time.
+      var active = 0;
+      var maxActive = 0;
+      final gates = <Completer<void>>[];
+      final lifecycle = DaemonLifecycle(
+        resolver: MakitCliResolver(
+          candidatePaths: const ['/x/makit'],
+          exists: (_) => true,
+          shellLookup: () async => null,
+        ),
+        run: (exe, args) async {
+          active++;
+          if (active > maxActive) maxActive = active;
+          final gate = Completer<void>();
+          gates.add(gate);
+          await gate.future;
+          active--;
+          return ProcessResult(0, 0, '', '');
+        },
+      );
+      final c = DesktopController(
+        client: _FakeControlClient(),
+        lifecycle: lifecycle,
+      );
+
+      // Fire two actions without awaiting them.
+      final f1 = c.start();
+      final f2 = c.stop();
+      await pumpEventQueue();
+
+      // Only the first action has begun; the second is queued behind it.
+      expect(gates.length, 1);
+      expect(maxActive, 1);
+
+      gates[0].complete();
+      await pumpEventQueue();
+      // Now the second action runs — still alone.
+      expect(gates.length, 2);
+      gates[1].complete();
+      await Future.wait([f1, f2]);
+
+      expect(maxActive, 1, reason: 'lifecycle actions must never overlap');
+    });
+
     test('notifies listeners on refresh', () async {
       var notes = 0;
       final c = DesktopController(

app/test/desktop/chat/server_profile_badge_test.dart

@@ -45,6 +45,9 @@ class _NoWriteFs extends FileSystemAdapter {
   String? readOrNull(String path) => null;
   @override
   void writeAtomic(String path, String contents) {}
+  // Without this, the base withLock creates `<path>.lock` on the real disk.
+  @override
+  T withLock<T>(String path, T Function() body) => body();
 }
 
 ProfileLifecycle _lifecycle({required bool targetRunning}) => ProfileLifecycle(

app/test/desktop/daemon/profile_lifecycle_test.dart

@@ -168,7 +168,7 @@ void main() {
         statusProbe: (_) async => true,
         // No pid file exists for this fake home, so confirmation rests on the
         // socket going away — exactly what this test covers.
-        readPid: (_) async => null,
+        readPid: (_) => null,
         sleep: (_) async {},
       );
 
@@ -248,7 +248,7 @@ void main() {
           resolver: _resolver(),
           run: _RecordingRunner().run,
           socketExists: (_) => false, // socket already gone
-          readPid: (_) async => 4242,
+          readPid: (_) => 4242,
           // Alive for the first two polls, then the process exits.
           processAlive: (pid) async {
             expect(pid, 4242);
@@ -278,7 +278,7 @@ void main() {
           resolver: _resolver(),
           run: _RecordingRunner().run,
           socketExists: (_) => false, // socket gone...
-          readPid: (_) async => 99,
+          readPid: (_) => 99,
           processAlive: (_) async => true, // ...but the process never exits
           sleep: (_) async {},
         );
@@ -306,7 +306,7 @@ void main() {
           probed = true;
           return false;
         },
-        readPid: (_) async => null,
+        readPid: (_) => null,
         sleep: (_) async {},
       );
 
@@ -328,7 +328,7 @@ void main() {
         resolver: _resolver(path: null),
         run: _RecordingRunner().run,
         socketExists: (_) => false,
-        readPid: (_) async => null,
+        readPid: (_) => null,
         sleep: (_) async {},
       );
 

app/test/desktop/daemon/profile_registry_test.dart

@@ -631,9 +631,10 @@ void main() {
         final pa = await a.createUserProfile(name: 'Alpha');
         final pb = await b.createUserProfile(name: 'Beta');
         expect(pa.port, pb.port, reason: 'the scenario needs them to collide');
+        final contestedPort = pa.port;
 
-        a.save();
-        b.save();
+        a.save(); // Alpha is persisted first, on the contested port.
+        b.save(); // Beta collides; the already-persisted Alpha must keep it.
 
         final onDisk = ProfileRegistry.load(
           makitRoot: kRoot,
@@ -646,6 +647,10 @@ void main() {
           ports.length,
           reason: 'no two profiles may share a port after the merge',
         );
+        // The on-disk (possibly-running) profile keeps its port; the newcomer
+        // yields.
+        expect(onDisk.byId(pa.id)!.port, contestedPort);
+        expect(onDisk.byId(pb.id)!.port, isNot(contestedPort));
       },
     );
     // The id is interpolated into a filesystem path (the secure-store namespace

app/test/desktop/daemon/profile_runtime_test.dart

@@ -9,6 +9,7 @@
 // ignore_for_file: depend_on_referenced_packages, invalid_use_of_visible_for_testing_member
 import 'dart:io';
 
+import 'package:flutter/foundation.dart' show FlutterError;
 import 'package:flutter_test/flutter_test.dart';
 import 'package:makit/desktop/daemon/daemon_lifecycle.dart';
 import 'package:makit/desktop/daemon/profile_lifecycle.dart';
@@ -189,5 +190,21 @@ void main() {
       );
       await expectLater(runtime.dispose(), completes);
     });
+
+    test('dispose disposes the profilesController (no leak per switch)', () async {
+      // profilesController is injected via overrideWithValue, which Riverpod does
+      // not dispose, so the runtime must. A disposed ChangeNotifier throws when a
+      // listener is added.
+      final runtime = ProfileRuntime.create(
+        profile: _target,
+        registry: _registry(),
+        prefs: await _prefs(),
+      );
+      await runtime.dispose();
+      expect(
+        () => runtime.profilesController.addListener(() {}),
+        throwsA(isA<FlutterError>()),
+      );
+    });
   });
 }

app/test/desktop/settings/profiles_section_test.dart

@@ -23,6 +23,11 @@ class _MemFs extends FileSystemAdapter {
   String? readOrNull(String path) => null;
   @override
   void writeAtomic(String path, String contents) {}
+  // Without this, the base withLock creates `<path>.lock` on the REAL disk
+  // (under a non-existent `/Users/test`), which breaks the no-disk guarantee and
+  // stalls the widget test's pumpAndSettle.
+  @override
+  T withLock<T>(String path, T Function() body) => body();
 }
 
 /// An in-memory [ProfileFileSystem] for the deleter: only the paths seeded in
@@ -442,6 +447,11 @@ void main() {
     await tester.pumpAndSettle();
     await tester.tap(find.widgetWithText(FilledButton, 'Delete profile'));
     await tester.pumpAndSettle();
+    // Advance the binding's (fake) clock so the delete's async work settles.
+    // A bare `await Future.delayed(...)` deadlocks here: inside `testWidgets`
+    // the clock only moves when the tester pumps.
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pumpAndSettle();
 
     final successes = center.events.where(
       (e) => e.severity == StatusSeverity.success,

app/test/desktop/settings/server_devices_section_test.dart

@@ -77,6 +77,7 @@ Future<void> _pump(
   MakitConnState? connection,
   StatusCenter? statusCenter,
   ServerProfile? profile,
+  NavigatorObserver? observer,
   bool tall = false,
 }) async {
   if (tall) {
@@ -100,7 +101,10 @@ Future<void> _pump(
         if (statusCenter != null)
           statusCenterProvider.overrideWithValue(statusCenter),
       ],
-      child: const MaterialApp(home: Scaffold(body: ServerDevicesSection())),
+      child: MaterialApp(
+        navigatorObservers: observer == null ? const [] : [observer],
+        home: const Scaffold(body: ServerDevicesSection()),
+      ),
     ),
   );
   await tester.pump();
@@ -408,29 +412,13 @@ void main() {
   testWidgets('nav rows disclose their content inline (no page push)', (
     tester,
   ) async {
-    final config = await makeConfig();
     final observer = _RecordingObserver();
-    await tester.pumpWidget(
-      ProviderScope(
-        overrides: [
-          serverConfigProvider.overrideWith((ref) => config),
-          desktopControllerProvider.overrideWithValue(_controller()),
-          connectionProvider.overrideWithValue(MakitConnState()),
-          controlClientProvider.overrideWithValue(
-            FakeControlClient(sessions: const []),
-          ),
-        ],
-        child: MaterialApp(
-          navigatorObservers: [observer],
-          home: const Scaffold(body: ServerDevicesSection()),
-        ),
-      ),
+    await _pump(
+      tester,
+      config: await makeConfig(),
+      observer: observer,
+      tall: true,
     );
-    tester.view.physicalSize = const Size(1200, 2400);
-    tester.view.devicePixelRatio = 1.0;
-    addTearDown(tester.view.resetPhysicalSize);
-    addTearDown(tester.view.resetDevicePixelRatio);
-    await tester.pump();
     observer.pushed.clear();
 
     expect(find.text('No running sessions'), findsNothing);