Files
logos-logoscore-cli/tests/test_local_endpoint.cpp
Dario Gabriel LipicarandClaude Opus 5 0f0be25959 fix(client): fail at once when the daemon is gone, and stop reporting that as data
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>
2026-08-23 11:33:56 -03:00

149 lines
4.8 KiB
C++

#include <gtest/gtest.h>
#include "local_endpoint.h"
#include <QDir>
#include <cstring>
#include <fstream>
#include <string>
#ifndef _WIN32
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#endif
// localEndpointProvablyAbsent() is what turns "a daemon that stopped" from a
// twenty-second wait into an immediate answer, and it is the one piece of this
// that could refuse a LIVE daemon if it got either half wrong. So these pin
// both halves: the path derivation (against QDir::tempPath(), which is what Qt
// resolves a bare QLocalSocket/QLocalServer name against) and the liveness
// verdict for each shape the path can be in.
namespace {
std::string uniqueId(const char* suffix)
{
return "ut" + std::to_string(::getpid()) + suffix;
}
QString endpointPath(const std::string& instanceId)
{
return QDir::tempPath()
+ QStringLiteral("/logos_core_service_")
+ QString::fromStdString(instanceId);
}
#ifndef _WIN32
// Bind and listen at `path`. Returns the fd, or -1. Closing the fd without
// unlinking leaves exactly what a hard-killed daemon leaves: a socket inode
// with nobody behind it.
int bindListen(const QString& path)
{
const std::string p = path.toStdString();
sockaddr_un addr{};
if (p.size() >= sizeof(addr.sun_path)) return -1;
addr.sun_family = AF_UNIX;
std::memcpy(addr.sun_path, p.c_str(), p.size());
const int fd = ::socket(AF_UNIX, SOCK_STREAM, 0);
if (fd < 0) return -1;
::unlink(p.c_str());
if (::bind(fd, reinterpret_cast<sockaddr*>(&addr), sizeof(addr)) != 0
|| ::listen(fd, 4) != 0) {
::close(fd);
return -1;
}
return fd;
}
#endif
} // namespace
TEST(LocalEndpointTest, ReportsTheDerivedPathItChecked)
{
#ifdef _WIN32
GTEST_SKIP() << "named pipes: no path is derived";
#else
std::string path;
logosctl::localEndpointProvablyAbsent("core_service", "abc123", &path);
EXPECT_EQ(path, endpointPath("abc123").toStdString())
<< "the path must be the one a bare QLocalSocket name resolves to, or "
"the check is answering a question about the wrong file";
#endif
}
TEST(LocalEndpointTest, NoSocketFileAtAll_IsProvablyAbsent)
{
// What a clean `daemon stop` leaves: QLocalServer's destructor unlinks it.
const std::string id = uniqueId("_gone");
QDir().remove(endpointPath(id));
#ifdef _WIN32
EXPECT_FALSE(logosctl::localEndpointProvablyAbsent("core_service", id));
#else
EXPECT_TRUE(logosctl::localEndpointProvablyAbsent("core_service", id));
#endif
}
#ifndef _WIN32
TEST(LocalEndpointTest, SocketFileWithNoListener_IsProvablyAbsent)
{
// The case a stat cannot answer, and the reason this does a connect at
// all. A hard-killed daemon leaves the inode behind, and even a clean stop
// leaves a window between the shutdown reply and the destructor running --
// which is exactly when someone types the next command.
const std::string id = uniqueId("_dead");
const QString path = endpointPath(id);
const int fd = bindListen(path);
ASSERT_GE(fd, 0) << "could not bind " << path.toStdString();
::close(fd); // listener gone, inode stays
ASSERT_TRUE(QDir().exists(path)) << "the socket file should have survived";
EXPECT_TRUE(logosctl::localEndpointProvablyAbsent("core_service", id))
<< "a socket file nobody is listening on is not a reachable daemon";
::unlink(path.toStdString().c_str());
}
TEST(LocalEndpointTest, LiveListener_IsNeverCalledAbsent)
{
// The control, and the one that matters most: refusing a reachable daemon
// is far worse than the wait this avoids.
const std::string id = uniqueId("_live");
const QString path = endpointPath(id);
const int fd = bindListen(path);
ASSERT_GE(fd, 0) << "could not bind " << path.toStdString();
EXPECT_FALSE(logosctl::localEndpointProvablyAbsent("core_service", id));
::close(fd);
::unlink(path.toStdString().c_str());
}
#endif
TEST(LocalEndpointTest, SomeOtherFileWearingTheName_IsNotEvidence)
{
// Only S_ISSOCK inodes get an opinion. A regular file that happens to
// match the name says nothing about any daemon.
const std::string id = uniqueId("_plain");
const QString path = endpointPath(id);
{ std::ofstream ofs(path.toStdString(), std::ios::trunc); ofs << "x"; }
ASSERT_TRUE(QDir().exists(path));
EXPECT_FALSE(logosctl::localEndpointProvablyAbsent("core_service", id));
QDir().remove(path);
}
TEST(LocalEndpointTest, NothingToDeriveANameFromIsNotEvidence)
{
// A remote dial spec carries no instance_id. That says nothing about any
// local socket, so it must not be read as "the endpoint is missing".
EXPECT_FALSE(logosctl::localEndpointProvablyAbsent("core_service", ""));
EXPECT_FALSE(logosctl::localEndpointProvablyAbsent("", "abc123"));
}