Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ app/test/sim/frames/

# Local QA artifacts (screenshots, issue logs) from tool/shoot-ports.sh
.qa/
.piano/

# Editor/agent scratch files must never be committed.
*.bak
*.orig

# Local pnpm store, created when a store-dir is set for this checkout
.pnpm-store/
43 changes: 31 additions & 12 deletions app/integration_test/desktop/control_e2e_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import 'package:makit/desktop/settings/sections/server_devices_section.dart';
import 'package:makit/desktop/settings/server_config.dart';
import 'package:makit/store/connection.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:makit/store/prefs/profile_scoped_prefs.dart';

const _socketPath = String.fromEnvironment('MAKIT_CONTROL_SOCK');
const _timeout = Duration(seconds: 20);
Expand Down Expand Up @@ -92,7 +93,10 @@ void main() {
controlClientProvider.overrideWithValue(client),
desktopControllerProvider.overrideWithValue(controller),
serverConfigProvider.overrideWith(
(ref) => ServerConfigController(prefs, const ServerConfig()),
(ref) => ServerConfigController(
ProfileScopedPrefs.unscoped(prefs),
const ServerConfig(),
),
),
connectionProvider.overrideWithValue(MakitConnState()),
],
Expand Down Expand Up @@ -139,7 +143,7 @@ void main() {
);
});

testWidgets('Endpoint bind-mode picker drives the unified ServerConfig', (
testWidgets('Reachability picker drives the unified ServerConfig', (
tester,
) async {
expect(
Expand All @@ -150,7 +154,10 @@ void main() {

SharedPreferences.setMockInitialValues({});
final prefs = await SharedPreferences.getInstance();
final config = ServerConfigController(prefs, const ServerConfig());
final config = ServerConfigController(
ProfileScopedPrefs.unscoped(prefs),
const ServerConfig(),
);

final client = ReconnectingControlClient(
create: () => MakitControlClient(socketPath: _socketPath),
Expand Down Expand Up @@ -187,16 +194,32 @@ void main() {
reason: 'lifecycle never showed a running daemon',
);

// Ships defaulting to Auto (the new secure default) with no host field.
expect(config.current.bindMode, ServerBindMode.auto);
// The real daemon is up (the active-profile row reads "Running").
await _pumpUntil(
tester,
find.text('Running'),
reason: 'active-profile row never showed a running daemon',
);

// Ships defaulting to "My devices" (the secure default), no host field.
expect(config.current.reachability, Reachability.myDevices);
expect(
find.ancestor(of: find.text('Host'), matching: find.byType(TextField)),
findsNothing,
);

// Selecting Custom reveals a host field and persists the mode.
await _scrollAndTap(tester, find.text('Custom'));
expect(config.current.bindMode, ServerBindMode.custom);
// Selecting "Just this Mac" pins loopback in serveArgs.
await _scrollAndTap(tester, find.text('Just this Mac'));
await tester.pumpAndSettle();
expect(config.current.reachability, Reachability.thisMacOnly);
expect(
config.current.serveArgs(),
containsAllInOrder(['--host', '127.0.0.1']),
);

// The custom-host escape hatch lives under Diagnostics → Advanced.
await _scrollAndTap(tester, find.text('Diagnostics'));
await _scrollAndTap(tester, find.text('Advanced'));
final host = find.ancestor(
of: find.text('Host'),
matching: find.byType(TextField),
Expand All @@ -212,9 +235,5 @@ void main() {
config.current.serveArgs(),
containsAllInOrder(['--host', '0.0.0.0']),
);

// Switching to Loopback persists too (no daemon restart is triggered).
await _scrollAndTap(tester, find.text('Loopback'));
expect(config.current.bindMode, ServerBindMode.loopback);
});
}
6 changes: 5 additions & 1 deletion app/integration_test/desktop/settings_repo_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import 'package:makit/desktop/settings/settings_window.dart';
import 'package:makit/store/connection.dart';
import 'package:makit/store/models.dart';
import 'package:makit/store/store.dart';
import 'package:makit/store/prefs/profile_scoped_prefs.dart';
import 'package:makit/ui/home/repo_monogram.dart';
import 'package:shared_preferences/shared_preferences.dart';

Expand Down Expand Up @@ -107,7 +108,10 @@ Widget _app() => ProviderScope(
overrides: [
reposProvider.overrideWithValue(ReposState(_repos)),
serverConfigProvider.overrideWith(
(ref) => ServerConfigController(_prefs, const ServerConfig()),
(ref) => ServerConfigController(
ProfileScopedPrefs.unscoped(_prefs),
const ServerConfig(),
),
),
desktopControllerProvider.overrideWithValue(
DesktopController(
Expand Down
16 changes: 16 additions & 0 deletions app/lib/control/reconnecting_control_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,25 @@ class ReconnectingControlClient implements ControlClient {
}

/// Disposes any live connection. Safe to call multiple times.
///
/// An in-flight connect is awaited first: `close()` only nulling `_current`
/// would let a pending `_connectNew` complete *after* close and install a live
/// socket into `_current` that nothing ever closes. This leaked the old
/// profile's connection on a switch, which then kept polling under a torn-down
/// runtime. Awaiting the pending connect lets it settle into `_current` (or
/// fail and dispose itself), after which the single disposal below covers it.
Future<void> close() async {
final connecting = _connecting;
if (connecting != null) {
try {
await connecting;
} catch (_) {
// A failed connect already disposed its client in _connectNew.
}
}
final current = _current;
_current = null;
_connecting = null;
if (current != null) await _safeDispose(current);
}

Expand Down
40 changes: 40 additions & 0 deletions app/lib/control/reconnecting_control_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -158,5 +158,45 @@ void main() {
expect(connected, [0, 1]);
},
);

test(
'close() disposes a client whose connect was still in flight',
() async {
// Regression: close() used to only null `_current`, so a connect still
// in flight would complete afterwards, install a live socket into
// `_current`, and leak it — the old profile's runtime kept polling after
// teardown.
final localCreated = <_FakeClient>[];
final localDisposed = <int>[];
final gate = Completer<void>();
var seq = 0;
final client = ReconnectingControlClient(
create: () {
final c = _FakeClient(seq++);
localCreated.add(c);
return c;
},
connect: (_) async => gate.future, // stays in flight until released
dispose: (c) async => localDisposed.add((c as _FakeClient).id),
);

// Trigger connect but do not await the call yet.
final pending = client.status();
await Future<void>.delayed(Duration.zero);
expect(localCreated, hasLength(1));

// Close while the connect is in flight, then let the connect complete.
final closing = client.close();
gate.complete();
await closing;
await pending.then((_) {}, onError: (_) {});

// The in-flight client must have been disposed, not retained.
expect(localDisposed, contains(0));
// And it is not reused: the next call connects a fresh client.
await client.status();
expect(localCreated, hasLength(2));
},
);
});
}
14 changes: 8 additions & 6 deletions app/lib/desktop/chat/groups/groups_controller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import 'dart:convert';

import 'package:flutter/foundation.dart';
import 'package:flutter_riverpod/legacy.dart';
import 'package:shared_preferences/shared_preferences.dart';

import '../../../store/prefs/profile_scoped_prefs.dart';

import '../panes/split_node.dart';
import '../panes/workspace_controller.dart';
Expand Down Expand Up @@ -200,10 +201,10 @@ class GroupsController extends StateNotifier<GroupsState> {

/// Builds a controller from persisted state, migrating the SPEC-28 single
/// workspace when this is the first run with groups.
static GroupsController load(SharedPreferences prefs) =>
static GroupsController load(ScopedPrefs prefs) =>
GroupsController(prefs, decode(prefs));

final SharedPreferences? _prefs;
final ScopedPrefs? _prefs;

/// The group with [id], or null.
Group? groupById(String id) {
Expand Down Expand Up @@ -506,7 +507,7 @@ class GroupsController extends StateNotifier<GroupsState> {
/// Decodes persisted state, migrating the SPEC-28 single workspace on first
/// run and falling back to [GroupsState.fresh] for anything unusable.
@visibleForTesting
static GroupsState decode(SharedPreferences prefs) {
static GroupsState decode(ScopedPrefs prefs) {
final raw = prefs.getString(kGroupsPrefsKey);
if (raw == null || raw.isEmpty) return _migrateLegacy(prefs);
Object? decoded;
Expand Down Expand Up @@ -579,7 +580,7 @@ class GroupsController extends StateNotifier<GroupsState> {
/// First run with groups: fold the SPEC-28 single workspace into one board so
/// nobody loses their layout. Empty tabs are carried over verbatim (decision
/// 21) and bound tabs become the board's membership.
static GroupsState _migrateLegacy(SharedPreferences prefs) {
static GroupsState _migrateLegacy(ScopedPrefs prefs) {
final legacyRaw = prefs.getString(kWorkspacePrefsKey);
if (legacyRaw == null || legacyRaw.isEmpty) return GroupsState.fresh();
final tree = WorkspaceController.decodeWorkspace(legacyRaw);
Expand Down Expand Up @@ -711,7 +712,8 @@ class GroupsController extends StateNotifier<GroupsState> {
}

/// The groups layer. Defaults to a non-persisting controller; `runDesktopApp`
/// overrides it with a [SharedPreferences]-backed one, and tests may too.
/// overrides it with a profile-scoped [ScopedPrefs]-backed one, and tests may
/// too.
final groupsControllerProvider =
StateNotifierProvider<GroupsController, GroupsState>(
(ref) => GroupsController.ephemeral(),
Expand Down
Loading
Loading