Skip to content

Commit 62f2339

Browse files
authored
FE integration verification + desktop full-stack e2e + daemon spawn-crash hardening (#30)
* fix(adapters): never let an agent spawn failure crash the daemon pi/acp/codex all spawned their agent subprocess with no child.on('error') handler. A spawn failure (ENOENT/EACCES) is delivered as an unlistened 'error' event, which Node re-throws as an uncaught exception — crashing the whole daemon and every other session with it (e.g. 'makit serve' dies with 'spawn pi ENOENT' when pi is not installed). Handle child 'error' (routed to the exit/teardown path so pending requests reject and an 'exited' status is surfaced), plus stdin/stdout/stderr 'error' (EPIPE writing to a dead agent, read faults), and guard stdin writes in try/catch. acp/codex defaultConnect are exported and use a settle-once+buffer so a spawn 'error' firing before onExit registers is not lost. Tests (TDD): spawn failure, stdin EPIPE, and sync-write-throw no longer crash the process; a missing codex binary makes start() reject cleanly. * test(e2e): fix mobile helper for onboarding gate + add desktop full-stack e2e Mobile: the Onboarding (#24) change inserts a skippable notifications step between pairing and Home. On a fresh simulator (permission notDetermined) the stub e2e suite got stuck there. launchMakit now dismisses that gate ('Not now') before waiting for the session list. No production/onboarding code touched. Desktop: add a full-stack control-plane e2e for the macOS control app — the counterpart to the mobile WS suite. server/test/e2e-control-server.ts serves a real daemon control socket backed by the StubAdapter (no real pi, no WS/TLS/ mDNS), seeding one device + one default session. control_e2e_test.dart pumps the real DesktopDashboard wired to a real ReconnectingControlClient and asserts status/devices.list/pair.mint/sessions.list round-trip. tool/e2e-desktop.sh orchestrates it (-d macos); integration-desktop-ci.yml runs it (workflow_dispatch). * fix(project-store): drop non-existent project dirs on load loadProjectPaths now filters out entries that no longer exist or aren't directories, so a stale/deleted project path isn't resurrected on restart. * docs(ci): document local-first e2e; mark macOS integration jobs manual The macOS-runner e2e suites are already workflow_dispatch-only (no per-PR cost). Make that intent explicit: add a CONTRIBUTING.md 'End-to-end tests (run locally)' section with the exact commands (app/tool/e2e.sh, app/tool/e2e-desktop.sh), and a LOCAL-FIRST note atop each manual workflow pointing at the local script. No behavior change — triggers are unchanged.
1 parent 10b2cac commit 62f2339

15 files changed

Lines changed: 719 additions & 14 deletions

File tree

.github/workflows/integration-ci.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ name: Integration E2E (stub adapter)
77
# MANUAL (workflow_dispatch) only — not a per-PR gate. The cheap Linux gates
88
# (server-ci unit tests + typecheck, protocol-contract, real-pi-pinned headless
99
# smoke) cover regressions on every PR. The real-pi path stays in real-pi-e2e.yml.
10+
#
11+
# LOCAL-FIRST: prefer running this locally before pushing —
12+
# app/tool/e2e.sh --mode=stub
13+
# (see CONTRIBUTING.md → "End-to-end tests"). This CI job is a convenience
14+
# escape hatch you trigger by hand from the Actions tab, not an automatic gate.
1015

1116
on:
1217
workflow_dispatch: {}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
name: Desktop Control E2E (stub adapter)
2+
3+
# Full-stack control-plane e2e for the macOS desktop control app (SPEC-03):
4+
# the real desktop app ↔ a real daemon control socket backed by the StubAdapter
5+
# (server/test/e2e-control-server.ts). No LLM, no `pi`, no WS — the control
6+
# socket (status/devices/sessions/pair/logs) is the whole surface under test.
7+
# Counterpart to integration-ci.yml, which exercises the mobile WS stack on an
8+
# iOS simulator. macOS runners are expensive, so this is MANUAL
9+
# (workflow_dispatch) only — the cheap Linux gates (server-ci, protocol-contract)
10+
# cover the control protocol contract on every PR.
11+
#
12+
# LOCAL-FIRST: prefer running this locally before pushing —
13+
# app/tool/e2e-desktop.sh
14+
# (see CONTRIBUTING.md → "End-to-end tests"). This CI job is a convenience
15+
# escape hatch you trigger by hand from the Actions tab, not an automatic gate.
16+
17+
on:
18+
workflow_dispatch: {}
19+
20+
jobs:
21+
desktop-e2e:
22+
runs-on: macos-latest
23+
timeout-minutes: 20
24+
steps:
25+
- name: Checkout code
26+
uses: actions/checkout@v5
27+
28+
- name: Setup Flutter
29+
uses: subosito/flutter-action@v2
30+
with:
31+
flutter-version: '3.44.4'
32+
channel: 'stable'
33+
cache: true
34+
35+
- name: Setup Node.js
36+
uses: actions/setup-node@v5
37+
with:
38+
node-version: '22'
39+
40+
- name: Enable Corepack
41+
run: corepack enable
42+
43+
- name: Prepare pnpm
44+
run: corepack prepare pnpm@11.8.0 --activate
45+
46+
- name: Cache pnpm store
47+
uses: actions/cache@v5
48+
with:
49+
path: ~/.local/share/pnpm/store
50+
key: ${{ runner.os }}-pnpm-${{ hashFiles('server/pnpm-lock.yaml') }}
51+
restore-keys: |
52+
${{ runner.os }}-pnpm-
53+
54+
- name: Cache Flutter packages
55+
uses: actions/cache@v5
56+
with:
57+
path: ~/.pub-cache
58+
key: ${{ runner.os }}-flutter-${{ hashFiles('app/pubspec.lock') }}
59+
restore-keys: |
60+
${{ runner.os }}-flutter-
61+
62+
# Native Xcode build products for the macOS desktop build.
63+
- name: Cache Xcode DerivedData
64+
uses: actions/cache@v5
65+
with:
66+
path: ~/Library/Developer/Xcode/DerivedData
67+
key: ${{ runner.os }}-deriveddata-macos-${{ hashFiles('app/pubspec.lock') }}
68+
restore-keys: |
69+
${{ runner.os }}-deriveddata-macos-
70+
71+
- name: Install server dependencies (lockfile-locked)
72+
working-directory: server
73+
run: pnpm secure:install
74+
75+
- name: Install app dependencies (lockfile-locked)
76+
working-directory: app
77+
run: flutter pub get --enforce-lockfile
78+
79+
- name: Desktop control-plane e2e (stub adapter)
80+
run: app/tool/e2e-desktop.sh

CONTRIBUTING.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,33 @@ covers all future contributions.
3434
4. Open a pull request describing **what** changed and **why**.
3535
5. Sign the CLA when the bot prompts you.
3636

37+
### End-to-end tests (run locally)
38+
39+
The full-stack e2e suites drive the real Flutter app against a real TLS/socket
40+
server. They need a macOS host (an iOS simulator or the macOS desktop build),
41+
so CI would run them on **expensive** macOS runners. To keep CI cheap they are
42+
**local-first**: not per-PR gates, only manually triggerable via
43+
`workflow_dispatch`. The cheap Linux gates (`server-ci`, `protocol-contract`,
44+
`real-pi-pinned`) cover regressions on every PR. Run the macOS suites locally
45+
before pushing changes that touch the app ↔ server boundary:
46+
47+
- **Mobile stub e2e** (real app ↔ TLS WS server, StubAdapter, iOS simulator):
48+
```sh
49+
cd server && pnpm secure:install # once
50+
cd app && flutter pub get # once
51+
app/tool/e2e.sh --mode=stub # pick a sim with MAKIT_SIM_NAME="iPhone 17 Pro"
52+
```
53+
- **Desktop control-plane e2e** (real macOS control app ↔ daemon control
54+
socket, StubAdapter):
55+
```sh
56+
app/tool/e2e-desktop.sh
57+
```
58+
- **Real-pi e2e** (genuine `pi` binary + local fake model) — optional, requires
59+
`pi` on `PATH`: `app/tool/e2e.sh --mode=real`.
60+
61+
The matching CI workflows (`integration-ci`, `integration-desktop-ci`, and the
62+
macOS job in `real-pi-e2e`) can also be run on demand from the Actions tab.
63+
3764
## Engineering standards
3865

3966
These are enforced (see [`AGENTS.md`](./AGENTS.md)):
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// Full-stack control-plane e2e for the macOS desktop control app (SPEC-03).
2+
//
3+
// Counterpart to the mobile stub suite (integration_test/stub/): instead of the
4+
// WS client, this drives the real desktop control screens against a real daemon
5+
// control socket served by `server/test/e2e-control-server.ts`. The whole stack
6+
// is genuine — `MakitControlClient` speaks the real NDJSON protocol over the
7+
// real unix socket to the real `createServerBackend`, and the real
8+
// `DesktopDashboard`/`DesktopController` render the responses.
9+
//
10+
// The socket path is injected by `app/tool/e2e-desktop.sh` via
11+
// `--dart-define=MAKIT_CONTROL_SOCK`. We wire the same client/controller
12+
// `runDesktopApp` builds, but pump `DesktopDashboard` directly so the test
13+
// needs no tray/window native plugins.
14+
//
15+
// ignore_for_file: depend_on_referenced_packages
16+
import 'package:flutter/material.dart';
17+
import 'package:flutter_riverpod/flutter_riverpod.dart';
18+
import 'package:flutter_test/flutter_test.dart';
19+
import 'package:integration_test/integration_test.dart';
20+
import 'package:makit/control/control_client.dart';
21+
import 'package:makit/control/reconnecting_control_client.dart';
22+
import 'package:makit/desktop/daemon/daemon_lifecycle.dart';
23+
import 'package:makit/desktop/desktop_app.dart';
24+
import 'package:makit/desktop/desktop_controller.dart';
25+
import 'package:makit/desktop/screens/providers.dart';
26+
27+
const _socketPath = String.fromEnvironment('MAKIT_CONTROL_SOCK');
28+
const _timeout = Duration(seconds: 20);
29+
30+
/// Pump at 100ms steps until [finder] matches or [_timeout] expires.
31+
Future<void> _pumpUntil(
32+
WidgetTester tester,
33+
Finder finder, {
34+
String? reason,
35+
}) async {
36+
final deadline = DateTime.now().add(_timeout);
37+
while (DateTime.now().isBefore(deadline)) {
38+
await tester.pump(const Duration(milliseconds: 100));
39+
if (finder.evaluate().isNotEmpty) {
40+
await tester.pump(const Duration(milliseconds: 100));
41+
return;
42+
}
43+
}
44+
fail(reason ?? 'timed out waiting for $finder');
45+
}
46+
47+
void main() {
48+
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
49+
50+
testWidgets('desktop dashboard drives a real daemon control socket', (
51+
tester,
52+
) async {
53+
expect(
54+
_socketPath.isNotEmpty,
55+
isTrue,
56+
reason: 'MAKIT_CONTROL_SOCK must be passed via --dart-define',
57+
);
58+
59+
final client = ReconnectingControlClient(
60+
create: () => MakitControlClient(socketPath: _socketPath),
61+
connect: (c) => (c as MakitControlClient).connect(),
62+
dispose: (c) => (c as MakitControlClient).dispose(),
63+
);
64+
final controller = DesktopController(
65+
client: client,
66+
lifecycle: DaemonLifecycle(resolver: MakitCliResolver()),
67+
);
68+
addTearDown(() async {
69+
controller.dispose();
70+
await client.close();
71+
});
72+
controller.startPolling();
73+
74+
await tester.pumpWidget(
75+
ProviderScope(
76+
overrides: [
77+
controlClientProvider.overrideWithValue(client),
78+
desktopControllerProvider.overrideWithValue(controller),
79+
],
80+
child: const MaterialApp(home: DesktopDashboard()),
81+
),
82+
);
83+
84+
// status → the header renders the live pid over the real socket.
85+
await _pumpUntil(
86+
tester,
87+
find.textContaining('Server running (pid'),
88+
reason:
89+
'header never showed a running daemon — control socket handshake '
90+
'or status verb failed',
91+
);
92+
93+
// devices.list → the seeded device shows on the (default) Devices tab.
94+
await _pumpUntil(
95+
tester,
96+
find.text('e2e phone'),
97+
reason: 'devices.list did not surface the seeded device',
98+
);
99+
100+
// pair.mint → the QR tab mints and renders a makit:// pair url.
101+
await tester.tap(find.text('Pair QR'));
102+
await _pumpUntil(
103+
tester,
104+
find.textContaining('makit://pair'),
105+
reason: 'pair.mint did not return a pair url',
106+
);
107+
108+
// sessions.list → the Sessions tab lists the seeded default session.
109+
await tester.tap(find.textContaining('Sessions ('));
110+
await _pumpUntil(
111+
tester,
112+
find.text('new session'),
113+
reason: 'sessions.list did not surface the default session',
114+
);
115+
});
116+
}

app/integration_test/e2e_helpers.dart

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ const _messageTimeout = Duration(seconds: 15);
1313
Future<void> launchMakit(WidgetTester tester) async {
1414
app.main();
1515
await tester.pump(const Duration(milliseconds: 100));
16+
// The test creds are seeded as paired, so onboarding skips the pair step —
17+
// but on a fresh simulator notification permission is notDetermined, so the
18+
// wizard now stops at the skippable notifications gate before Home. Dismiss
19+
// it ("Not now") so the suite reaches the session list as before.
20+
await _skipNotificationsStep(tester);
1621
await pumpUntil(
1722
tester,
1823
find.text('new session'),
@@ -23,6 +28,23 @@ Future<void> launchMakit(WidgetTester tester) async {
2328
);
2429
}
2530

31+
/// Dismiss the notifications onboarding gate if it's showing. No-op once the
32+
/// app has already advanced past it, so it's safe to call unconditionally.
33+
Future<void> _skipNotificationsStep(WidgetTester tester) async {
34+
final skip = find.widgetWithText(TextButton, 'Not now');
35+
final deadline = DateTime.now().add(_connectionTimeout);
36+
while (DateTime.now().isBefore(deadline)) {
37+
await tester.pump(const Duration(milliseconds: 100));
38+
if (skip.evaluate().isNotEmpty) {
39+
await tester.tap(skip);
40+
await tester.pump(const Duration(milliseconds: 100));
41+
return;
42+
}
43+
// Already past the gate (session list rendering) — nothing to skip.
44+
if (find.text('new session').evaluate().isNotEmpty) return;
45+
}
46+
}
47+
2648
/// Open the first session in the list (the stub server pre-creates one
2749
/// "new session" entry).
2850
Future<void> openFirstSession(WidgetTester tester) async {

app/tool/e2e-desktop.sh

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
# Full-stack desktop control-plane e2e (SPEC-03). Boots the real daemon control
5+
# socket (server/test/e2e-control-server.ts, StubAdapter — no real pi) and runs
6+
# the real macOS desktop app against it (integration_test/desktop/). Counterpart
7+
# to tool/e2e.sh, which does the mobile WS stack on an iOS simulator.
8+
#
9+
# macOS-only: the desktop control app is the `Platform.isMacOS` branch of
10+
# main.dart, so the harness targets the `macos` device, not a simulator.
11+
12+
ROOT="$(cd "$(dirname "$0")/../.." && pwd)"
13+
SERVER_DIR="$ROOT/server"
14+
APP_DIR="$ROOT/app"
15+
16+
FLUTTER_BIN="${MAKIT_FLUTTER_BIN:-$(command -v flutter || true)}"
17+
if [[ -z "$FLUTTER_BIN" ]]; then
18+
echo "flutter not found — set MAKIT_FLUTTER_BIN or add flutter to PATH" >&2
19+
exit 1
20+
fi
21+
FLUTTER_BIN_DIR="$(cd "$(dirname "$FLUTTER_BIN")" && pwd)"
22+
23+
# Per-run private MAKIT_HOME so the harness never touches the real ~/.makit.
24+
export MAKIT_HOME="$(mktemp -d -t makit-desktop-e2e.XXXXXX)"
25+
SERVER_LOG="$(mktemp -t makit-desktop-e2e-server.XXXXXX.log)"
26+
SERVER_PID=""
27+
28+
cleanup() {
29+
if [[ -n "$SERVER_PID" ]] && kill -0 "$SERVER_PID" >/dev/null 2>&1; then
30+
kill "$SERVER_PID" >/dev/null 2>&1 || true
31+
wait "$SERVER_PID" >/dev/null 2>&1 || true
32+
fi
33+
rm -rf "$MAKIT_HOME" >/dev/null 2>&1 || true
34+
}
35+
trap cleanup EXIT INT TERM
36+
37+
(
38+
cd "$SERVER_DIR"
39+
pnpm exec tsx test/e2e-control-server.ts
40+
) >"$SERVER_LOG" 2>&1 &
41+
SERVER_PID="$!"
42+
43+
READY_JSON=""
44+
for _ in {1..200}; do
45+
if ! kill -0 "$SERVER_PID" >/dev/null 2>&1; then
46+
echo "control-server exited before ready; log:" >&2
47+
cat "$SERVER_LOG" >&2
48+
exit 1
49+
fi
50+
READY_JSON="$(grep -m1 '"ready":true' "$SERVER_LOG" || true)"
51+
if [[ -n "$READY_JSON" ]]; then
52+
break
53+
fi
54+
sleep 0.1
55+
done
56+
57+
if [[ -z "$READY_JSON" ]]; then
58+
echo "timed out waiting for control-server; log:" >&2
59+
cat "$SERVER_LOG" >&2
60+
exit 1
61+
fi
62+
63+
SOCK="$(node -e 'const fs=require("fs"); const j=JSON.parse(fs.readFileSync(0,"utf8")); process.stdout.write(j.socket)' <<<"$READY_JSON")"
64+
65+
cd "$APP_DIR"
66+
set +e
67+
PATH="$FLUTTER_BIN_DIR:$PATH" "$FLUTTER_BIN" test integration_test/desktop/control_e2e_test.dart \
68+
-d macos \
69+
--dart-define=MAKIT_CONTROL_SOCK="$SOCK"
70+
flutter_exit=$?
71+
set -e
72+
73+
if (( flutter_exit != 0 )); then
74+
echo "control-server log: $SERVER_LOG" >&2
75+
tail -100 "$SERVER_LOG" >&2 || true
76+
fi
77+
exit "$flutter_exit"

server/src/adapters/acp.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
type PromptRequest,
1515
type PromptResponse,
1616
} from "@agentclientprotocol/sdk";
17-
import { AcpAdapter, type AcpTransport } from "./acp.js";
17+
import { AcpAdapter, defaultConnect, type AcpTransport } from "./acp.js";
1818
import type { AdapterEvent } from "./adapter.js";
1919
import type { UICall, UIResponse } from "../uicall.js";
2020

@@ -421,3 +421,18 @@ test("emits exit + exited status on kill", async () => {
421421

422422
assert.ok(events.some((e) => e.kind === "session.status" && (e.payload as any).status === "exited"));
423423
});
424+
425+
test("acp defaultConnect routes a spawn failure to onExit instead of crashing the daemon", async () => {
426+
const transport = defaultConnect({
427+
agent: "test",
428+
command: "makit-nonexistent-binary-xyz",
429+
})(process.cwd(), {});
430+
const exit = new Promise<number | null>((resolve) => transport.onExit(resolve));
431+
const code = await Promise.race([
432+
exit,
433+
new Promise<number | null>((_, reject) =>
434+
setTimeout(() => reject(new Error("onExit never fired on spawn failure")), 2000).unref(),
435+
),
436+
]);
437+
assert.equal(code, null);
438+
});

0 commit comments

Comments
 (0)