mirror of
https://github.com/logos-co/logos-logoscore-cli.git
synced 2026-08-31 04:41:06 +00:00
Dario Gabriel LipicarandClaude Opus 5
0f0be25959
Against a session whose daemon is no longer there, `logosctl module ls` waited 22 seconds and then printed `[]` and exited 0. Not "failed slowly" -- reported success, with an empty module list, about a daemon that did not exist. `stats` did the same. `call`, `package`, `catalog` and `key` waited the same 20 seconds before reporting RPC_FAILED. Only `stop` and `status` were quick, because #100 gave them a guard the other fourteen commands never got. The mechanism is the one #100 diagnosed. A LocalSocket client "connects" to a socket path with no listener without complaint, QtRO surfaces no transport error for an absent peer, and the request is therefore neither answered nor refused -- so it waits out Timeout(20000) (logos-protocol, cpp/logos_mode.h) and a dead daemon is indistinguishable from a slow one until the deadline fires. Connecting is not the check it looks like. A session outlives its daemon in two shapes, and they need different evidence. CRASHED SESSION. daemon/state.json is still on disk naming a pid that is gone. This is #100's check, and it was copied into stop_command and status_command. It now lives in one place -- detectStaleSession(), called from Command::ensureConnected() -- which is the single door every RPC-opening command goes through, so all of them inherit it instead of the two that had it hand-written. StatusCommand still calls the helper itself, one step earlier, because its answer to "no daemon" is a status report rather than an error. #100's instance_id gate is preserved exactly: the guard fires only when the state file describes the daemon THIS client dials. A remote client can have a co-resident daemon's leftovers sitting in its own session directory, and its dial spec carries no instance_id at all, so an empty one never matches. The liveness syscall now runs before the client-config read, so the common path (daemon running) does not parse client/config.yaml twice per command. STOPPED SESSION. The tidier way to get here, and the one the pid guard cannot see: a clean `daemon stop` REMOVES daemon/state.json, leaving client/config.yaml and the token behind with no pid left to find dead. Every command still waited the full 20s. RpcClient::connect() now asks the socket instead, before it builds a LogosAPIClient (localEndpointProvablyAbsent, src/local_endpoint.h): the dial resolves to QDir::tempPath()/logos_core_service_<instance_id>, because the SDK asks for the bare name (LogosInstance::id) and Qt resolves a bare QLocalSocket/QLocalServer name against the temp dir. Deriving it the same way is what makes the answer sound rather than a guess. A stat alone is NOT enough, which cost this patch a wrong first draft. The socket file outlives the daemon: a hard kill leaves it, and a clean stop leaves it for the window between the shutdown reply and QLocalServer's destructor -- which is exactly when the next command gets typed. Measured through the new CLI sweep, stat-only vs stat-plus-connect over the same abandoned socket: 85.3s (every command timed out) vs 0.8s. So presence settles nothing and being REFUSED does; ECONNREFUSED is the same signal logos::isSocketDead uses to decide a socket is safe for the daemon's boot reaper to unlink. That function is not reused directly only because it sits behind the logos-protocol link, which logosctl_testlib deliberately does without. The check fails closed on everything short of proof: a socket that accepts us, any other connect() error, a path too long for sun_path, a non-socket inode, a tcp/tcp_ssl dial, an empty instance_id, Windows (named pipes, no inode). Refusing a reachable daemon would be far worse than the wait being removed. AN UNANSWERED QUERY IS NOT AN EMPTY ONE. The exit-0 half is a separate defect and survives independently of the timing: listModules() and getModuleStats() answered a failed RPC with LogosList::array(), the only two calls in the client that reported failure as data. Both now return optional<LogosList>, and the commands report DAEMON_UNREACHABLE with exit 2. `status` had the same shape by a different route -- RpcClient::getStatus synthesises a not_running report and marks it `rpc_error`, and that report has a "daemon" key, so it reached the success branch and exited 0 while printing "not running". It exits 1 now, as docs/project.md always said it did. `status` also connects directly rather than through ensureConnected(): that helper PRINTS a NO_DAEMON envelope, and letting it do so put two JSON documents on stdout for one command, which no `jq` invocation survives. Nothing opts out of the guard. `watch` is the one command with a case for waiting -- a daemon that has not started yet is a reasonable thing to watch for -- but it does no waiting today: it connects once and gives up, so failing in milliseconds is what it already meant to do. The four commands the issue listed that are NOT covered (`token issue|revoke|list`, `daemon|client config`) never call ensureConnected at all: they read and write the session's own files and have no daemon to be absent. TESTS. * CLITest.{Crashed,CleanlyStopped}Session_EveryRpcCommandFailsAtOnce and SocketLeftOverWithNoListener_EveryRpcCommandFailsAtOnce: all 17 commands against all three shapes, end-to-end, killed at 5s so exit 124 means the command was still waiting. Driven against the pre-fix binary via $LOGOSCTL_BINARY these fail with 124 on 15 of 17 commands, 80.3s. * CLITest.*_StatusReportsNotRunningAtOnce: exit 1, names the pid where there is one, and exactly one JSON document. * CommandTest.EveryRpcCommand_*: the 17 commands x 4 session shapes, against a mock, asserting on connectAttempts/rpcCalls -- a guard that fired is visible as the ABSENCE of contact. Three of the four shapes are the controls: live pid, foreign instance_id, and no state file at all must still dial. * LocalEndpointTest.*: the path derivation against QDir::tempPath(), plus a verdict for each shape the path can be in -- missing, socket with no listener, LIVE listener, and a regular file wearing the name. * CommandTest.{ListModules,Stats}_{UnansweredRpc,AnsweredWithNothing}_* and Status_{UnansweredRpc,LiveDaemon}_*: both sides of the empty-vs-unanswered line. CommandTest had no Status_ coverage at all, which is how exit 0 survived. Before/after over the shipped binaries, same stale session, macOS: module ls exit 0 after 22s printing [] -> exit 2 in <1s, names the pid stats exit 0 after 20s printing [] -> exit 2 in <1s status exit 0 after 20s -> exit 1 in <1s call/package/catalog/key 20s, RPC_FAILED -> exit 2 in <1s and against a cleanly stopped session, where nothing was fast before, all of the above are now under a second too. Live-daemon behaviour is unchanged and checked: 249 unit + 30 CLI + 25 integration tests pass for logosctl and 20 CLI + 24 integration for logoscore via `nix build .#checks.<sys>.tests-logosctl` / `-logoscore`. The 25 integration tests drive real daemons through logosctl, so a wrong socket path would fail them loudly rather than silently refusing live sessions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>