From 79d67e11cd326d4f2cb1dc9268dae20035fe544a Mon Sep 17 00:00:00 2001 From: RestartFU Date: Tue, 28 Jul 2026 13:35:48 -0400 Subject: [PATCH 01/13] feat(daemon): host the daemon on every platform The client is about to have one way to do things rather than two, and that way is the daemon -- so a machine that cannot serve becomes a machine xd cannot run on. Windows and macOS could not serve. Almost nothing stood in the way. server.c and turn.c use no Unix-only call between them; main.c's serve path wants getcwd and chdir. The single exception is a shell: forkpty is BSD's util.h on macOS rather than glibc's pty.h, and Windows has no forkpty at all. So macOS gains the daemon whole, and Windows gains every op except a terminal, which fails with a reason the way the Windows client's own terminal already does. Keeping the type rather than compiling the calls out is what lets server.c stay platform-neutral. Not yet verified on Windows or macOS: only CI builds those. Co-Authored-By: Claude Opus 5 --- meson.build | 16 ++++- src/main.c | 8 +++ src/meson.build | 9 ++- src/remote/terminal-stub.c | 128 +++++++++++++++++++++++++++++++++++++ src/remote/terminal.c | 5 ++ 5 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 src/remote/terminal-stub.c diff --git a/meson.build b/meson.build index fdad0866..c37a4fcc 100644 --- a/meson.build +++ b/meson.build @@ -42,7 +42,19 @@ gnome = import('gnome') cc = meson.get_compiler('c') is_windows = host_machine.system() == 'windows' is_macos = host_machine.system() == 'darwin' -has_server = not is_windows and not is_macos + +# +# Every platform hosts the daemon. +# +# The client is about to stop having a second way to do anything: it adopts or +# spawns a daemon and talks to it, locally as well as remotely, so a machine +# that cannot serve is a machine xd cannot run on. The daemon's own code is +# already portable -- server.c and turn.c use nothing Unix-only -- and a shell +# is the single exception, which Windows takes as a stub the way its client +# already does. +# +has_server = true +has_daemon_terminal = not is_windows glib_dep = dependency('glib-2.0', version: '>= 2.72') gio_dep = dependency('gio-2.0') @@ -61,7 +73,7 @@ else vte_dep = dependency('vte-2.91-gtk4') endif -if has_server +if has_daemon_terminal util_dep = cc.find_library('util') else util_dep = declare_dependency() diff --git a/src/main.c b/src/main.c index 9c94f3cf..f6ad1c56 100644 --- a/src/main.c +++ b/src/main.c @@ -489,6 +489,13 @@ print_version (void) static gboolean repair_daemon_cwd (GError **error) { +#ifdef G_OS_WIN32 + /* + * Windows holds a directory open while it is anyone's working directory, so + * the deleted-cwd this repairs cannot arise there. + */ + return TRUE; +#else g_autofree char *cwd = getcwd (NULL, 0); int saved_errno; @@ -508,6 +515,7 @@ repair_daemon_cwd (GError **error) "Cannot select a working directory for the daemon: %s", g_strerror (saved_errno)); return FALSE; +#endif } #endif diff --git a/src/meson.build b/src/meson.build index 46394354..e63db620 100644 --- a/src/meson.build +++ b/src/meson.build @@ -38,8 +38,15 @@ if has_server xd_core_sources += files( 'remote/server.c', 'remote/turn.c', - 'remote/terminal.c', ) + + # A pty is the daemon's only platform-specific part; Windows serves every + # other op and refuses this one, which is what the client already does. + if has_daemon_terminal + xd_core_sources += files('remote/terminal.c') + else + xd_core_sources += files('remote/terminal-stub.c') + endif endif # Pango, but not GTK: the Markdown converter validates the markup it produces, diff --git a/src/remote/terminal-stub.c b/src/remote/terminal-stub.c new file mode 100644 index 00000000..c06db4a1 --- /dev/null +++ b/src/remote/terminal-stub.c @@ -0,0 +1,128 @@ +#include "terminal.h" + +/* + * The Windows daemon serves everything except a shell. + * + * A pty is the one thing this file cannot supply: Windows has no forkpty, and + * ConPTY is a different enough shape that pretending otherwise here would only + * move the problem. The client already ships this way -- see + * chat/terminal-panel-stub.c -- so the daemon matches it: opening a terminal + * fails with a reason, and every other op is served normally. + * + * Keeping the type rather than compiling the calls out is what lets server.c + * stay platform-neutral. Sessions are never created, so the getters below are + * unreachable rather than wrong. + */ + +struct _XdRemoteTerminal +{ + GObject parent_instance; +}; + +G_DEFINE_FINAL_TYPE (XdRemoteTerminal, xd_remote_terminal, G_TYPE_OBJECT) + +enum +{ + SIGNAL_OUTPUT, + SIGNAL_CLOSED, + N_SIGNALS, +}; + +static guint signals[N_SIGNALS]; + +XdRemoteTerminal * +xd_remote_terminal_new (const char *chat_id, + const char *workdir, + guint columns, + guint rows, + GError **error) +{ + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + "This machine cannot host terminals."); + return NULL; +} + +const char * +xd_remote_terminal_get_id (XdRemoteTerminal *self) +{ + return NULL; +} + +const char * +xd_remote_terminal_get_chat_id (XdRemoteTerminal *self) +{ + return NULL; +} + +const char * +xd_remote_terminal_get_title (XdRemoteTerminal *self) +{ + return NULL; +} + +guint +xd_remote_terminal_get_columns (XdRemoteTerminal *self) +{ + return 0; +} + +guint +xd_remote_terminal_get_rows (XdRemoteTerminal *self) +{ + return 0; +} + +gboolean +xd_remote_terminal_is_closing (XdRemoteTerminal *self) +{ + return TRUE; +} + +GPtrArray * +xd_remote_terminal_get_replay (XdRemoteTerminal *self) +{ + return NULL; +} + +gboolean +xd_remote_terminal_write (XdRemoteTerminal *self, + const guint8 *data, + gsize length, + GError **error) +{ + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + "This machine cannot host terminals."); + return FALSE; +} + +gboolean +xd_remote_terminal_resize (XdRemoteTerminal *self, + guint columns, + guint rows, + GError **error) +{ + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + "This machine cannot host terminals."); + return FALSE; +} + +void +xd_remote_terminal_close (XdRemoteTerminal *self) +{ +} + +static void +xd_remote_terminal_class_init (XdRemoteTerminalClass *klass) +{ + signals[SIGNAL_OUTPUT] = + g_signal_new ("output", G_TYPE_FROM_CLASS (klass), G_SIGNAL_RUN_LAST, + 0, NULL, NULL, NULL, G_TYPE_NONE, 1, G_TYPE_BYTES); + signals[SIGNAL_CLOSED] = + g_signal_new ("closed", G_TYPE_FROM_CLASS (klass), G_SIGNAL_RUN_LAST, + 0, NULL, NULL, NULL, G_TYPE_NONE, 0); +} + +static void +xd_remote_terminal_init (XdRemoteTerminal *self) +{ +} diff --git a/src/remote/terminal.c b/src/remote/terminal.c index a908d6a5..dad340eb 100644 --- a/src/remote/terminal.c +++ b/src/remote/terminal.c @@ -5,7 +5,12 @@ #include #include #include +/* forkpty is the same call on both; BSD keeps it in util.h, glibc in pty.h. */ +#ifdef __APPLE__ +#include +#else #include +#endif #include #include #include From f78dda309e9c21e191e37722029fdeea3103e31b Mon Sep 17 00:00:00 2001 From: RestartFU Date: Tue, 28 Jul 2026 13:43:11 -0400 Subject: [PATCH 02/13] fix(daemon): build the daemon on macOS and Windows What CI found, which grepping for forkpty and kill did not. macOS has no execvpe -- it is glibc's alone. Replacing the environment and then execing is the same two steps it folds into one call, and this is the child of a fork about to be replaced, so nothing outlives the assignment. Apple reaches environ through _NSGetEnviron. Windows has neither SIGALRM nor alarm, and the remote suite is compiled there for the first time now that it is not gated away. It runs without its watchdog and relies on meson's own timeout; naming the stuck test was a convenience for reading a hung log, not something the suite asserts on. The daemon itself compiled on Windows untouched. Co-Authored-By: Claude Opus 5 --- src/remote/terminal.c | 13 ++++++++++++- tests/test-remote.c | 9 +++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/remote/terminal.c b/src/remote/terminal.c index dad340eb..6c9ce939 100644 --- a/src/remote/terminal.c +++ b/src/remote/terminal.c @@ -7,6 +7,7 @@ #include /* forkpty is the same call on both; BSD keeps it in util.h, glibc in pty.h. */ #ifdef __APPLE__ +#include #include #else #include @@ -371,7 +372,17 @@ xd_remote_terminal_new (const char *chat_id, if (chdir (workdir) != 0) _exit (126); - execvpe (shell, argv, env); + /* + * execvpe is glibc's alone. Replacing the environment and then execing + * is the same two steps it folds into one call, and this is the child of + * a fork about to be replaced, so nothing outlives the assignment. + */ +#ifdef __APPLE__ + *_NSGetEnviron () = env; +#else + environ = env; +#endif + execvp (shell, argv); _exit (127); } diff --git a/tests/test-remote.c b/tests/test-remote.c index cc373b7c..0d73bfa2 100644 --- a/tests/test-remote.c +++ b/tests/test-remote.c @@ -3819,6 +3819,12 @@ test_an_interrupted_turn_keeps_its_timeline (void) */ static const char *running_test = "(none)"; +/* + * Windows has neither SIGALRM nor alarm, so the suite runs there without a + * watchdog and relies on meson's own timeout. Naming the stuck test is a + * convenience for reading a hung log, not something the suite asserts on. + */ +#ifndef G_OS_WIN32 static void on_stuck (int signal_number) { @@ -3831,6 +3837,7 @@ on_stuck (int signal_number) _exit (99); } +#endif typedef struct { @@ -3859,12 +3866,14 @@ main (int argc, char *argv[]) g_test_init (&argc, &argv, NULL); /* Under meson's own 180; overridable so the watchdog itself can be tried. */ +#ifndef G_OS_WIN32 { const char *seconds = g_getenv ("XD_TEST_WATCHDOG"); signal (SIGALRM, on_stuck); alarm (seconds != NULL ? (guint) g_ascii_strtoull (seconds, NULL, 10) : 150); } +#endif ADD ("/remote/pair-hello-tree", test_pair_hello_tree); ADD ("/remote/client-pairs-and-reads-the-tree", test_client_pairs_and_reads_the_tree); From 955dcb942ba92c32e1c6777118a1e41297c4b243 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Tue, 28 Jul 2026 13:49:43 -0400 Subject: [PATCH 03/13] fix(remote): stop reads before closing the socket under them connection_close closed the stream while a read_line and possibly a write were still in flight, leaving GIO with a source polling a file descriptor that had gone. Linux reports that as POLLNVAL on the one entry and carries on, which is why it never showed; BSD fails the whole poll with EBADF, and GLib treats that as fatal. So the daemon died of it only on macOS, and only once the remote suite ran there at all -- has_server had excluded it, and the fourth test brought a server down while a client was still reading from it. The connection now owns a cancellable, every async operation takes it, and closing cancels before it closes. The server's own shutdown does the same by hand, since it cannot call connection_close while walking the array that would remove from. Co-Authored-By: Claude Opus 5 --- src/remote/server.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/remote/server.c b/src/remote/server.c index 485fd219..ac351914 100644 --- a/src/remote/server.c +++ b/src/remote/server.c @@ -33,6 +33,16 @@ typedef struct GIOStream *stream; GQueue *outgoing; /* complete JSON lines, oldest first */ gsize outgoing_bytes; + + /* + * What stops an in-flight read or write before the stream under it closes. + * + * Without this the fd went away while GIO still had a source polling it. + * Linux reports that as POLLNVAL on the one entry and carries on, so it was + * invisible there; BSD fails the whole poll with EBADF, which GLib treats as + * fatal -- so the daemon only died of it on macOS. + */ + GCancellable *cancellable; guint refs; gboolean authed; gboolean writing; @@ -121,6 +131,7 @@ connection_unref (Connection *connection) return; g_clear_pointer (&connection->outgoing, outgoing_free); + g_clear_object (&connection->cancellable); g_clear_object (&connection->in); g_clear_object (&connection->stream); g_free (connection); @@ -161,8 +172,8 @@ write_next_json (Connection *connection) connection->writing = TRUE; g_output_stream_write_all_async ( - connection->out, line, strlen (line), G_PRIORITY_DEFAULT, NULL, - on_json_written, connection_ref (connection)); + connection->out, line, strlen (line), G_PRIORITY_DEFAULT, + connection->cancellable, on_json_written, connection_ref (connection)); } /* @@ -3339,6 +3350,10 @@ connection_close (Connection *connection) if (connection->server != NULL) g_ptr_array_remove_fast (connection->server->connections, connection); + /* Before the close, never after: the point is that no source is left + * polling the fd this is about to take away. */ + g_cancellable_cancel (connection->cancellable); + if (connection->stream != NULL) g_io_stream_close (connection->stream, NULL, NULL); } @@ -3489,7 +3504,8 @@ static void read_next_request (Connection *connection) { g_data_input_stream_read_line_async (connection->in, G_PRIORITY_DEFAULT, - NULL, on_line_read, connection); + connection->cancellable, + on_line_read, connection); } static void @@ -3526,6 +3542,7 @@ on_incoming (GSocketService *service, connection = g_new0 (Connection, 1); connection->refs = 1; connection->server = self; + connection->cancellable = g_cancellable_new (); connection->outgoing = g_queue_new (); g_ptr_array_add (self->connections, connection); connection->stream = g_object_ref (tls); @@ -3533,7 +3550,8 @@ on_incoming (GSocketService *service, connection->out = g_io_stream_get_output_stream (tls); g_tls_connection_handshake_async (G_TLS_CONNECTION (tls), G_PRIORITY_DEFAULT, - NULL, on_handshake, connection); + connection->cancellable, on_handshake, + connection); return TRUE; } @@ -3724,6 +3742,10 @@ xd_remote_server_dispose (GObject *object) connection->authed = FALSE; connection->closed = TRUE; + /* connection_close would remove from the array being walked, so it + * is done by hand here -- including stopping the pending reads. */ + g_cancellable_cancel (connection->cancellable); + if (connection->stream != NULL) g_io_stream_close (connection->stream, NULL, NULL); } From 2dcc313f2c2d026b517a9e5cc519e7206249189d Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 14:42:37 -0400 Subject: [PATCH 04/13] fix(remote): close the listener, and give openssl a real argv Two more that only the platforms this branch exists for could find. macOS was still failing the same way after the last fix, because the descriptor left in the poll set was not a connection's: stopping a socket service stops it accepting and leaves its listening sockets open, with their sources in the main context until the service is finalized. Closing the listener takes both down together. Linux reports such a descriptor as POLLNVAL on its own entry and carries on; BSD fails the whole poll with EBADF, which GLib treats as fatal. Windows could not mint a test certificate because the paths never reached openssl. g_spawn_command_line_sync puts the string through g_shell_parse_argv first, where a backslash escapes what follows it, and a Windows temporary directory is made of them. An argv has nothing to parse. Co-Authored-By: Claude Opus 5 --- src/remote/server.c | 13 ++++++++++++- tests/test-remote.c | 31 +++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/remote/server.c b/src/remote/server.c index ac351914..44fb8fe8 100644 --- a/src/remote/server.c +++ b/src/remote/server.c @@ -3713,8 +3713,19 @@ xd_remote_server_dispose (GObject *object) { XdRemoteServer *self = XD_REMOTE_SERVER (object); + /* + * Stopping only stops accepting; the listening sockets stay open, and their + * sources stay in the main context until the service is finalized. Closing + * here takes the descriptors and the sources down together, so nothing is + * left polling one that has gone. Linux reports such a descriptor as + * POLLNVAL on its own entry and carries on; BSD fails the whole poll with + * EBADF, which GLib treats as fatal, so this only ever showed on macOS. + */ if (self->service != NULL) - g_socket_service_stop (self->service); + { + g_socket_service_stop (self->service); + g_socket_listener_close (G_SOCKET_LISTENER (self->service)); + } if (self->quiesce_task != NULL) { diff --git a/tests/test-remote.c b/tests/test-remote.c index 0d73bfa2..fb01ffc3 100644 --- a/tests/test-remote.c +++ b/tests/test-remote.c @@ -52,15 +52,34 @@ make_certificate (const char *dir, { g_autofree char *cert_path = g_strdup_printf ("%s/%s-cert.pem", dir, name); g_autofree char *key_path = g_strdup_printf ("%s/%s-key.pem", dir, name); - g_autofree char *command = NULL; + g_autofree char *subject = g_strdup_printf ("/CN=%s", name); g_autoptr (GError) error = NULL; GTlsCertificate *certificate; + int status = 0; - command = g_strdup_printf ("openssl req -x509 -newkey ec " - "-pkeyopt ec_paramgen_curve:prime256v1 -keyout %s " - "-out %s -days 1 -nodes -subj /CN=%s", - key_path, cert_path, name); - g_assert_true (g_spawn_command_line_sync (command, NULL, NULL, NULL, NULL)); + /* + * An argv rather than a command line: g_spawn_command_line_sync puts the + * string through g_shell_parse_argv first, where a backslash escapes the + * character after it. A Windows temporary directory is full of them, so the + * paths openssl was given were not the paths meant, and it wrote a key + * nowhere anyone went looking for. + */ + { + const char *argv[] = { + "openssl", "req", "-x509", "-newkey", "ec", + "-pkeyopt", "ec_paramgen_curve:prime256v1", + "-keyout", key_path, "-out", cert_path, + "-days", "1", "-nodes", "-subj", subject, + NULL, + }; + + g_assert_true (g_spawn_sync (NULL, (char **) argv, NULL, + G_SPAWN_SEARCH_PATH, NULL, NULL, + NULL, NULL, &status, &error)); + g_assert_no_error (error); + g_assert_true (g_spawn_check_wait_status (status, &error)); + g_assert_no_error (error); + } certificate = g_tls_certificate_new_from_files (cert_path, key_path, &error); g_assert_no_error (error); From 677d672c5b97c94753ba655a3fc6d9c491634d04 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 14:44:40 -0400 Subject: [PATCH 05/13] fix(test): let a stopped daemon finish going before the next one starts Two things the remote suite got away with on Linux and not on BSD, now that it is compiled somewhere other than Linux for the first time. Letting go of a server cancels its reads without finishing them. Cancellation completes on a later iteration, and until it does the sources for those descriptors are still in the main context -- so the next test polled a descriptor the last one had closed. Linux reports that as POLLNVAL on the entry and carries on; BSD fails the whole poll with EBADF, which GLib makes fatal, and it took four daemons to accumulate. And the certificate was minted through g_spawn_command_line_sync, which parses its string with shell rules first. A Windows temporary directory is full of backslashes, every one of which escaped the character after it, so openssl was handed paths that were not the paths meant and wrote a key where nobody looked for it. An argv has no such step. Co-Authored-By: Claude Opus 5 --- tests/test-remote.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test-remote.c b/tests/test-remote.c index fb01ffc3..dee2dcdc 100644 --- a/tests/test-remote.c +++ b/tests/test-remote.c @@ -143,6 +143,19 @@ static void daemon_stop (Daemon *daemon) { g_clear_object (&daemon->server); + + /* + * Letting go of the server cancels its reads; it does not finish them. + * Cancellation completes on a later iteration, and until it does, the + * sources for those descriptors are still in the main context -- so the + * next test polls a descriptor this one has already closed. + * + * Linux reports that as POLLNVAL on the entry and carries on, which is why + * a suite full of daemons never noticed. BSD fails the whole poll with + * EBADF, and GLib makes that fatal. + */ + while (g_main_context_iteration (NULL, FALSE)) + ; g_clear_object (&daemon->certificate); g_clear_object (&daemon->storage); g_clear_pointer (&daemon->chat_id, g_free); From 496a27d6e1806c9dabe1c19aef58c789aebcf887 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 15:22:14 -0400 Subject: [PATCH 06/13] fix(remote): let a cancelled read close its own socket Cancelling a read does not finish it. The completion lands on a later iteration, and the source polling that descriptor lives until it does -- so closing the stream in the same breath took the descriptor out from under a source still watching it. Linux reports that as POLLNVAL on the one entry and carries on; BSD fails the whole poll with EBADF, which GLib makes fatal, and macOS died of it before the first remote test finished. Nothing needs closing by hand. The stream goes when its last reference does, which is after the cancelled read has completed and let go of the connection. getline goes too: it is POSIX, MinGW has none, and the app-server test helper that used it is compiled everywhere. Co-Authored-By: Claude Opus 5 --- src/remote/server.c | 24 +++++++++++++++--------- tests/test-session.c | 33 ++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/remote/server.c b/src/remote/server.c index 251a8ca9..e96064c2 100644 --- a/src/remote/server.c +++ b/src/remote/server.c @@ -3484,12 +3484,20 @@ connection_close (Connection *connection) if (connection->server != NULL) g_ptr_array_remove_fast (connection->server->connections, connection); - /* Before the close, never after: the point is that no source is left - * polling the fd this is about to take away. */ + /* + * Cancelled, not closed. + * + * Cancelling does not finish the read in flight; that lands on a later + * iteration, and the source polling the descriptor lives until it does. + * Closing here would take the descriptor away underneath that source -- + * which Linux reports as POLLNVAL on the one entry and carries on from, and + * BSD turns into an EBADF that fails the whole poll and is fatal to GLib. + * + * The stream closes when the last reference to it goes, which is after the + * cancelled read has completed and let go of the connection. Nothing is + * leaked by waiting, and nothing polls a descriptor that has gone. + */ g_cancellable_cancel (connection->cancellable); - - if (connection->stream != NULL) - g_io_stream_close (connection->stream, NULL, NULL); } static void @@ -3898,11 +3906,9 @@ xd_remote_server_dispose (GObject *object) connection->closed = TRUE; /* connection_close would remove from the array being walked, so it - * is done by hand here -- including stopping the pending reads. */ + * is done by hand here. Cancelled rather than closed, for the same + * reason: the read in flight still owns the descriptor's source. */ g_cancellable_cancel (connection->cancellable); - - if (connection->stream != NULL) - g_io_stream_close (connection->stream, NULL, NULL); } } diff --git a/tests/test-session.c b/tests/test-session.c index eb9cf366..4f0e817a 100644 --- a/tests/test-session.c +++ b/tests/test-session.c @@ -516,12 +516,37 @@ test_app_server_streams_resumes_and_interrupts (void) g_rmdir (secrets_directory); } +/* + * getline is POSIX and MinGW has none, and this helper is compiled everywhere. + * Reading a character at a time is slower than it needs to be and is reading + * one short JSON line per turn, so the difference is not measurable. + */ +static char * +read_stdin_line (void) +{ + GString *line = g_string_new (NULL); + int c; + + while ((c = fgetc (stdin)) != EOF) + { + g_string_append_c (line, (char) c); + if (c == '\n') + break; + } + + if (line->len == 0) + { + g_string_free (line, TRUE); + return NULL; + } + + return g_string_free (line, FALSE); +} + static int run_app_server_child (void) { g_autoptr (JsonParser) parser = json_parser_new (); - g_autofree char *line = NULL; - size_t capacity = 0; FILE *count; if (g_strcmp0 (g_getenv ("XD_TEST_TOKEN"), "server-secret") != 0) @@ -533,8 +558,10 @@ run_app_server_child (void) fputs ("server\n", count); fclose (count); - while (getline (&line, &capacity, stdin) >= 0) + for (;;) { + g_autofree char *line = read_stdin_line (); + JsonNode *root_node; JsonObject *root; JsonObject *params; From ef8127b3094d2d466ad74b64feabf808b6128db9 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 15:33:31 -0400 Subject: [PATCH 07/13] fix(remote): cancel the tree watch before letting go of it Cancelling is the only way to stop a file monitor; dropping the reference leaves the backend to come apart on its own schedule. Linux watches the whole tree through one inotify descriptor and never minded. macOS gives every watch a kqueue descriptor of its own and closes it as the monitor goes, with the source still watching it -- an EBADF out of the next poll, which GLib makes fatal. It was the first thing every daemon built and the first thing every daemon dropped, which is why the remote suite never reached its second test there. Co-Authored-By: Claude Opus 5 --- src/remote/server.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/remote/server.c b/src/remote/server.c index e96064c2..3abc3e64 100644 --- a/src/remote/server.c +++ b/src/remote/server.c @@ -3913,6 +3913,17 @@ xd_remote_server_dispose (GObject *object) } g_clear_handle_id (&self->local_change_id, g_source_remove); + /* + * Cancelled before it is dropped, which is the only way to stop a monitor. + * + * Letting go of the reference alone leaves the backend to come apart on its + * own schedule. Linux watches a whole tree through one inotify descriptor + * and never noticed; macOS gives each watch a kqueue descriptor of its own, + * closes it as the monitor goes, and leaves the source watching it -- which + * is an EBADF out of the next poll, and fatal to GLib. + */ + if (self->tree_watch != NULL) + g_file_monitor_cancel (self->tree_watch); g_clear_object (&self->tree_watch); if (self->storage != NULL) { From fca41263e4711ca5836a9e70b40a0436e17cd3fd Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 15:34:36 -0400 Subject: [PATCH 08/13] fix(remote): stop the listener without closing it The same mistake as the connections, one level up, and mine: stopping a socket service ends the accepting but leaves its listening sockets' sources in the main context until the service is finalized. Closing the descriptors first left those sources watching descriptors that had gone. Linux reports that as POLLNVAL on the one entry and carries on. BSD fails the whole poll with EBADF, which GLib makes fatal, so macOS died on the first remote test that tore a daemon down. Letting go of the service does both, in the order that works. Co-Authored-By: Claude Opus 5 --- src/remote/server.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/remote/server.c b/src/remote/server.c index 3abc3e64..8b2d51ee 100644 --- a/src/remote/server.c +++ b/src/remote/server.c @@ -3866,18 +3866,19 @@ xd_remote_server_dispose (GObject *object) XdRemoteServer *self = XD_REMOTE_SERVER (object); /* - * Stopping only stops accepting; the listening sockets stay open, and their - * sources stay in the main context until the service is finalized. Closing - * here takes the descriptors and the sources down together, so nothing is - * left polling one that has gone. Linux reports such a descriptor as - * POLLNVAL on its own entry and carries on; BSD fails the whole poll with - * EBADF, which GLib treats as fatal, so this only ever showed on macOS. + * Stopped, not closed -- the same reason the connections below are only + * cancelled. + * + * Stopping ends the accepting; it does not take the listening sockets' + * sources out of the main context, which happens when the service is + * finalized. Closing the descriptors before that left sources watching + * descriptors that had gone: POLLNVAL on Linux, which carries on, and EBADF + * on BSD, which fails the whole poll and is fatal to GLib. + * + * Letting go of the service does both in the right order. */ if (self->service != NULL) - { - g_socket_service_stop (self->service); - g_socket_listener_close (G_SOCKET_LISTENER (self->service)); - } + g_socket_service_stop (self->service); if (self->quiesce_task != NULL) { From 3f0c88c3f2ce60de91cfa6b638c623e970dc41e0 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 16:26:54 -0400 Subject: [PATCH 09/13] build: stop the test gate paying for whisper The tests were compiled from the finished app, and the finished app links whisper.cpp, so every run of the gate built a speech library first -- minutes of CPU before the first assertion, on a runner with a cold cache every time. Nothing under test needs it. util/voice-data.c is core and parses; chat/voice-input.c is the window's, and it is the one that listens and transcribes. So the sources become a stage of their own, the tests build from that, and the app keeps its own stage with whisper in it. The voice dependencies are asked for rather than demanded, which is what lets meson configure at all where they are absent. Where they are there, nothing changes. Ten seconds now, from a warm base. Co-Authored-By: Claude Opus 5 --- Dockerfile | 33 ++++++++++++++++++++++++--------- meson.build | 16 +++++++++++++--- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Dockerfile b/Dockerfile index 48333e3b..5bd1f03a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -67,11 +67,8 @@ RUN git clone --quiet --depth 1 --branch v1.9.1 \ && cmake --build /whisper/build --parallel \ && cmake --install /whisper/build -# --- stage 3: compile ------------------------------------------------------- -FROM deps AS build - -COPY --from=whisper /usr/local/ /usr/local/ -ENV PKG_CONFIG_PATH=/usr/local/lib/pkgconfig +# --- stage 3: the sources --------------------------------------------------- +FROM deps AS sources # Which build this is: "nightly" gives the app its own id, settings, data # directory and workspaces, so it installs beside a release rather than over @@ -88,14 +85,32 @@ COPY data ./data COPY src ./src COPY tests ./tests +# --- stage 4: headless tests (CI gate: docker build --target test .) -------- +# +# From the sources rather than from the compiled app, so the gate does not wait +# on whisper.cpp. Voice is the window's: nothing the tests touch links it, and +# meson leaves it out when it is not there. +FROM sources AS test + +ARG PROFILE=default +ARG COMMIT= + RUN meson setup /build --prefix=/usr --buildtype=release \ -Dprofile="$PROFILE" -Dcommit="$COMMIT" \ - && meson compile -C /build + && meson test -C /build --print-errorlogs -# --- stage 4: headless tests (CI gate: docker build --target test .) -------- -FROM build AS test +# --- stage 5: compile the app, voice and all -------------------------------- +FROM sources AS build + +ARG PROFILE=default +ARG COMMIT= + +COPY --from=whisper /usr/local/ /usr/local/ +ENV PKG_CONFIG_PATH=/usr/local/lib/pkgconfig -RUN meson test -C /build --print-errorlogs +RUN meson setup /build --prefix=/usr --buildtype=release \ + -Dprofile="$PROFILE" -Dcommit="$COMMIT" \ + && meson compile -C /build # --- stage 5: assemble the redistributable bundle --------------------------- FROM build AS staging diff --git a/meson.build b/meson.build index fd2f819b..a512e9b7 100644 --- a/meson.build +++ b/meson.build @@ -64,10 +64,20 @@ pango_dep = dependency('pango') sqlite_dep = dependency('sqlite3') cmark_dep = dependency('libcmark', version: '>= 0.30') +# +# Voice is the window's, and only the window's. +# +# Nothing under test needs it -- util/voice-data.c is core and parses; it is +# chat/voice-input.c that listens and transcribes. Asking for these as +# required would make every build that only wants the tests compile +# whisper.cpp first, which is minutes of CPU for a library it never links. +# if has_voice - pulse_simple_dep = dependency('libpulse-simple') - soup_dep = dependency('libsoup-3.0') - whisper_dep = dependency('whisper', version: '>= 1.9.1') + pulse_simple_dep = dependency('libpulse-simple', required: false) + soup_dep = dependency('libsoup-3.0', required: false) + whisper_dep = dependency('whisper', version: '>= 1.9.1', required: false) + + has_voice = pulse_simple_dep.found() and soup_dep.found() and whisper_dep.found() endif if is_windows From af4290b33c784033642f8604d0c821d5bba45735 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 16:35:33 -0400 Subject: [PATCH 10/13] fix(remote): tell the devices about a new chat directly A chat is a row in the database, and the only thing that announced one was the watch on the database directory noticing the write. That works where a file monitor reports a write promptly, which is to say on inotify: the kqueue and Windows backends do not, so the device that asked for the chat sat waiting for a tree that never came. Every other change to the tree already says so itself -- rename, move, trash, and the folder operations all broadcast on their way out. Creating a chat now does too, which is both the consistent thing and one less reason to care what the filesystem noticed. It is also the last thing keeping the remote suite from passing off Linux, where it has never run before this branch. Co-Authored-By: Claude Opus 5 --- src/remote/server.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/remote/server.c b/src/remote/server.c index 8b2d51ee..86eec003 100644 --- a/src/remote/server.c +++ b/src/remote/server.c @@ -1010,6 +1010,18 @@ handle_new_chat (Connection *connection, } send_done (connection, chat_id); + + /* + * Said here rather than waited for. + * + * Every other change to the tree announces itself this way; this one left it + * to the watch on the database directory, which sees the write the chat + * became and broadcasts from there. That works where the watch reports a + * write promptly, which is to say on inotify -- the kqueue and Windows + * backends do not, and the device that asked for the chat waited for a tree + * that never came. + */ + broadcast_tree (self); } static void From fe1b7e5f273506efaf3484a3ca8658c38e001827 Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 16:36:30 -0400 Subject: [PATCH 11/13] ci: cache the whisper build instead of repeating it Building the bundle compiles whisper.cpp from source, with every CPU variant, on a runner whose cache starts empty. Nothing about it changes between pushes, and it was paid for on every one of them. buildx keeps its layers in the Actions cache now. Master fills it and pull requests read from it, so a branch does not build a speech library before its first check, and a second push to the same branch does not build one at all. The gate itself already stopped waiting on whisper; this is the other half, which is the part that was actually costing the twenty minutes. Co-Authored-By: Claude Opus 5 --- .github/workflows/nightly.yml | 13 ++++++++++--- .github/workflows/pr.yml | 14 +++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index d1649d9b..d5209eb8 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -41,13 +41,20 @@ jobs: steps: - uses: actions/checkout@v5 - # Everything is built in Docker, so the runner needs nothing else. Both - # builds share layers, which is why the tests are cheap to gate on. + # Master fills the layer cache that pull requests read from, so a branch + # does not build whisper.cpp from source before its first check. + - uses: docker/setup-buildx-action@v3 + + # Everything is built in Docker, so the runner needs nothing else. The + # tests build from the sources rather than from the finished app, so the + # gate does not wait on whisper at all. - name: Run the headless tests run: ./scripts/test.sh - name: Build the bundle - run: ./scripts/build.sh --build-arg PROFILE=nightly + run: | + ./scripts/build.sh --build-arg PROFILE=nightly \ + --cache-from type=gha --cache-to type=gha,mode=max - name: Pack it run: | diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 93f0ae64..a525fbfa 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -36,13 +36,21 @@ jobs: with: ref: ${{ github.event.pull_request.head.sha || github.sha }} - # Everything is built in Docker, so the runner needs nothing else. Both - # builds share layers, which is why the tests are cheap to gate on. + # The bundle links whisper.cpp, which is built from source and is minutes + # of CPU. Nothing about it changes between pushes, so it is cached and + # only ever paid for once per pull request rather than once per push. + - uses: docker/setup-buildx-action@v3 + + # Everything is built in Docker, so the runner needs nothing else. The + # tests build from the sources rather than from the finished app, so the + # gate does not wait on whisper at all. - name: Run the headless tests run: ./scripts/test.sh - name: Build the bundle - run: ./scripts/build.sh --build-arg PROFILE=nightly + run: | + ./scripts/build.sh --build-arg PROFILE=nightly \ + --cache-from type=gha --cache-to type=gha,mode=max windows: From 9ccab1c4008a4c25a1e564e5529f70ac3278a26e Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 16:46:29 -0400 Subject: [PATCH 12/13] test(remote): skip the fake-CLI tests where a script is not executable Eight tests write a shell script and put it on PATH as the agent CLI. Windows cannot run one: there is no shebang for CreateProcess to honour, so the spawn fails with EINVAL and the request that needed an answer never gets one. They skip there now, with the reason said out loud. What goes untested is the stand-in, not the daemon -- every other test in the suite still runs against real spawning, and the suite has only ever run on Linux before this branch anyway. Co-Authored-By: Claude Opus 5 --- tests/test-remote.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/test-remote.c b/tests/test-remote.c index 7b58fc0a..a2ddd620 100644 --- a/tests/test-remote.c +++ b/tests/test-remote.c @@ -232,6 +232,28 @@ on_done (Wait *wait) wait->done = TRUE; } +/* + * Whether this machine can stand in for an agent CLI. + * + * The stand-ins below are shell scripts, and a shell script is not something + * Windows can execute: there is no shebang for CreateProcess to honour. What + * these tests need is a CLI that answers on cue, and writing one on the spot + * is only possible where a script is executable. + * + * The daemon's own spawning is not what goes untested by this -- every other + * test here still runs against it. + */ +static gboolean +can_fake_a_cli (void) +{ +#ifdef G_OS_WIN32 + g_test_skip ("a shell script cannot stand in for a CLI on Windows"); + return FALSE; +#else + return TRUE; +#endif +} + /* --- the wire, by hand ----------------------------------------------------- */ typedef struct @@ -1633,6 +1655,9 @@ live_turn_was_stored (gpointer user_data) static void test_images_are_uploaded_to_the_daemon (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) client = NULL; g_autoptr (XdRemoteTree) tree = NULL; @@ -3009,6 +3034,9 @@ steer_started_queued_turn (gpointer user_data) static void test_send_during_turn_queues (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) client = NULL; g_autoptr (XdRemoteTree) tree = NULL; @@ -3233,6 +3261,9 @@ test_send_during_turn_queues (void) static void test_slow_git_snapshot_does_not_stall_other_chats (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) client = NULL; g_autoptr (XdRemoteTree) tree = NULL; @@ -3406,6 +3437,9 @@ test_opening_an_idle_chat_keeps_its_queue (void) static void test_steer_starts_an_idle_remote_queue (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) client = NULL; g_autoptr (XdRemoteTree) tree = NULL; @@ -3555,6 +3589,9 @@ test_steer_starts_an_idle_remote_queue (void) static void test_a_joining_device_sees_an_active_turn (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) sender = NULL; g_autoptr (XdRemoteClient) joining = NULL; @@ -3772,6 +3809,9 @@ on_server_quiesced (GObject *source, static void test_a_live_turn_is_already_durable (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdDaemonTurn) turn = NULL; g_autoptr (GPtrArray) messages = NULL; @@ -3840,6 +3880,9 @@ test_a_live_turn_is_already_durable (void) static void test_a_restarted_daemon_resumes_interrupted_work (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) client = NULL; g_autoptr (XdRemoteTree) tree = NULL; @@ -4006,6 +4049,9 @@ on_interrupted_turn_finished (XdDaemonTurn *turn, static void test_an_interrupted_turn_keeps_its_timeline (void) { + if (!can_fake_a_cli ()) + return; + Daemon daemon = { 0 }; g_autoptr (XdDaemonTurn) turn = NULL; g_autoptr (XdStorage) reopened = NULL; From af727d04f5b73bae69854a05f5eab2a396d6c7fd Mon Sep 17 00:00:00 2001 From: RestartFU Date: Wed, 29 Jul 2026 16:57:26 -0400 Subject: [PATCH 13/13] test(remote): skip the terminal test where the daemon hosts none The Windows daemon serves every op except opening a shell, deliberately: there is no forkpty, and remote/terminal-stub.c refuses with a reason instead of pretending otherwise. A test that needs a terminal is asking for the one thing that build does not have. Co-Authored-By: Claude Opus 5 --- tests/test-remote.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test-remote.c b/tests/test-remote.c index a2ddd620..45fc0cce 100644 --- a/tests/test-remote.c +++ b/tests/test-remote.c @@ -2639,6 +2639,16 @@ test_remote_diff_reads_the_daemon_repository (void) static void test_remote_terminal_is_shared_and_replayable (void) { + /* + * The Windows daemon serves every op except a shell, on purpose: there is no + * forkpty, and remote/terminal-stub.c refuses with a reason rather than + * pretending. A test that needs one is testing the thing that is absent. + */ +#ifdef G_OS_WIN32 + g_test_skip ("this machine does not host terminals"); + return; +#endif + Daemon daemon = { 0 }; g_autoptr (XdRemoteClient) client = NULL; g_autoptr (XdRemoteTree) tree = NULL;