feat(app-server): unix-socket transport + daemon/attach advertisement - #5749
feat(app-server): unix-socket transport + daemon/attach advertisement#5749Hmbown wants to merge 5 commits into
Conversation
…im handshake Desktop Phase 0, socket half (backlog #36; DESKTOP-APP-BRIEF §2/§5, CORE-PROTOCOL §5 attachment styles). `daemon_socket` serves the existing stdio JSON-RPC protocol verbatim over a user-private unix socket: - path resolution under the Codewhale runtime dir (explicit > $CODEWHALE_HOME/run > $XDG_RUNTIME_DIR/codewhale > macOS Application Support > ~/.codewhale/run), kernel path-length check, dir 0700, socket 0600, peer-uid check on accept; - stale-socket cleanup on start: dead socket reclaimed, live daemon refused (AlreadyRunning), non-socket file never touched (NotASocket); - `daemon/attach` claim handshake: attach vs claim, exclusive owner slot that frees on disconnect, bundle-skew guard via expect_daemon_version, typed errors -32010..-32014; only the owner may `shutdown`, enforced in the loop (also mid-turn) so a guest can neither stop the daemon nor interrupt turns; - Windows: named pipe reserved, bind/run return a typed UnsupportedPlatform error — no silent fallback; - run_stdio_loop gains a per-connection policy (transport label + shutdown authority) and a typed exit reason so the daemon knows an owner asked it to stop; stdio behavior is unchanged. Integration tests bind a daemon, connect over the socket, complete the handshake, round-trip app/* requests through the shared dispatcher, exercise guest/owner/relaunch claim semantics, version skew, stale cleanup, and clean shutdown (socket removed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aDEyM2a4pPZ9qqMDrP5YX Signed-off-by: CodeWhale Bot <bot@codewhale.net>
… socket docs Wire the daemon socket transport into the app-server subcommand (mutually exclusive with --stdio/--http/--mobile) and document endpoint resolution, the daemon/attach handshake, ownership rules, and the Windows named-pipe reservation in docs/RUNTIME_API.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aDEyM2a4pPZ9qqMDrP5YX Signed-off-by: CodeWhale Bot <bot@codewhale.net>
… stay warning-free
ATTACH_REQUIRED_CODE / DAEMON_ALREADY_CLAIMED_CODE / DAEMON_VERSION_SKEW_CODE
and JsonRpcError::{attach_required, daemon_already_claimed,
daemon_version_skew} are only reached from daemon_socket::platform, which is
#[cfg(unix)]. On cfg(not(unix)) rustc flags them dead, and the workspace's
`warnings = "deny"` (plus CI's -Dwarnings) turns that into a build failure on
the Windows leg. Gate the three consts and ctors on unix; the remaining daemon
errors (not_daemon_owner, already_attached) are referenced unconditionally.
Proven with a scratch crate mirroring the shape: the pre-fix shape errors
with four dead-code diagnostics under --target x86_64-pc-windows-msvc, the
gated shape checks clean. A direct app-server Windows check is still blocked
on this host by ring's build script (missing Windows C headers).
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
…den socket/CLI tests - `capabilities` on the unix-socket transport now lists `daemon/attach` right after `healthz`; the stdio pin (EXPECTED_CAPABILITY_METHODS) is untouched and a new unit test pins the socket variant. - AlreadyRunning wording no longer claims the live listener is a codewhale daemon (it may be anything bound at an explicit --socket-path). - Integration tests: pre-attach gating proven for capabilities, thread/list and prompt/run; post-attach capabilities checked; a still-attached guest is proven to receive EOF when the owner shuts the daemon down. - CLI: --socket conflicts with --stdio/--http/--mobile are unit-tested, and --socket-path without --socket is proven to be a MissingRequiredArgument. - docs/RUNTIME_API.md: capabilities note for the socket transport and the updated AlreadyRunning message. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @Hmbown's task in 4m 17s —— View job Review:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67aee8935f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let exit = run_stdio_loop(&context.state, lines, writer, policy).await; | ||
| drop(guard); |
There was a problem hiding this comment.
When the owner disconnects while a thread/message is still running, this await retains the ConnectionGuard until the turn finishes; run_stdio_loop merely sets stdin_open = false on EOF and continues awaiting the dispatch. If the turn is stalled and emits no further output, a relaunched desktop remains locked out with daemon_already_claimed, and falling back to guest attachment also prevents it from shutting the daemon down. Release the owner claim when EOF is observed rather than only after the active turn completes.
Useful? React with 👍 / 👎.
| let outcome = runtime.block_on(run_daemon_socket(DaemonSocketOptions { | ||
| socket_path: args.socket_path, | ||
| config_path: args.config, | ||
| })); |
There was a problem hiding this comment.
For codewhale --config /path/custom.toml app-server --socket, the dispatcher loads the global config and even uses it for telemetry, but this branch passes only the subcommand-level args.config, which is None. The daemon therefore silently starts from the default config instead of the explicitly selected file; combine this with cli.config as the surrounding telemetry path already does.
Useful? React with 👍 / 👎.
| Ok(Err(_refused)) => { | ||
| std::fs::remove_file(path).map_err(|source| DaemonSocketError::Io { |
There was a problem hiding this comment.
This branch treats every UnixStream::connect error as proof that the socket is stale. Transient errors such as a saturated Unix-socket backlog (EAGAIN) can occur while a listener is still alive, so a second startup can unlink the live daemon's endpoint and then create a split-brain listener. Only ConnectionRefused should authorize stale cleanup; other errors should leave the socket untouched and be reported.
Useful? React with 👍 / 👎.
| "app/capabilities" => { | ||
| dispatch_stdio_app_request(state, AppRequest::Capabilities, transport).await? | ||
| } |
There was a problem hiding this comment.
On an attached socket connection, app/capabilities is now dispatched with AppTransport::Socket, but process_app_request ignores that argument and still returns the hard-coded value "transport": "stdio+http". Socket clients using this scoped capability method therefore receive a response claiming their active transport is unsupported; include unix-socket or derive the field from the supplied transport.
Useful? React with 👍 / 👎.
| let xdg_runtime_dir = std::env::var_os("XDG_RUNTIME_DIR") | ||
| .filter(|value| !value.is_empty()) | ||
| .map(PathBuf::from); |
There was a problem hiding this comment.
If XDG_RUNTIME_DIR is set to a relative value, this accepts it verbatim and resolves the daemon endpoint relative to the process working directory. The CLI and desktop shell can then discover different sockets depending on where each was launched, allowing multiple unintended daemons and making attach fail despite a live process. Treat a relative XDG runtime directory as invalid or unset, consistent with the repository's absolute-path validation for global environment overrides.
Useful? React with 👍 / 👎.
| let state = build_state_with_transport(options.config_path, None, AppTransport::Socket) | ||
| .map_err(DaemonSocketError::State)?; |
There was a problem hiding this comment.
With the documented default invocation codewhale app-server --socket, options.config_path is None, so build_state_with_transport discards the resolved default config path and persist_config later treats every successful app/config/set or app/config/unset as an in-memory-only change. The socket client receives ok: true, but the setting disappears when this long-lived daemon restarts; retain the resolved store path for the daemon so advertised configuration mutations are durable.
Useful? React with 👍 / 👎.
1 participant
What
App-server unix-socket transport and daemon/attach advertisement (desktop Phase 0 foundation).
Evidence
Unproven
No-Issue: desktop Phase 0 socket listener