From e4c702a691edea1d99aa43e22fb797e192566f7f Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Mon, 3 Aug 2026 09:40:44 +0200 Subject: [PATCH] gui: use immutable ID aliases in 9P paths Connection monitoring and all 9P operations now use immutable session/agent UUIDs in paths instead of mutable display names. The server resolves these via the new Aliases mechanism in findChild. This fixes the disconnected-indicator bug after session rename: connections no longer go stale when names change. Removed dead sessionNameForId/agentNameForId helpers and the m_agentConnectionPaths tracking (unnecessary with stable paths). Rename operations no longer restart streams. --- gui/ollie9pclient.cpp | 152 +++++++++++------------------------------- gui/ollie9pclient.h | 2 - 2 files changed, 39 insertions(+), 115 deletions(-) diff --git a/gui/ollie9pclient.cpp b/gui/ollie9pclient.cpp index 231e121..18bb4b7 100644 --- a/gui/ollie9pclient.cpp +++ b/gui/ollie9pclient.cpp @@ -127,30 +127,6 @@ void Ollie9pClient::setActiveSessionId(const QString &id) emit activeStateChanged(); } -QString Ollie9pClient::sessionNameForId(const QString &sessionId) const -{ - for (const QVariant &value : std::as_const(m_sessions)) { - const QVariantMap session = value.toMap(); - if (session.value("id").toString() == sessionId) - return session.value("name").toString(); - } - return {}; -} - -QString Ollie9pClient::agentNameForId(const QString &sessionId, const QString &agentId) const -{ - for (const QVariant &value : std::as_const(m_sessions)) { - const QVariantMap session = value.toMap(); - if (session.value("id").toString() != sessionId) continue; - for (const QVariant &agentValue : session.value("agents").toList()) { - const QVariantMap agent = agentValue.toMap(); - if (agent.value("id").toString() == agentId) - return agent.value("name").toString(); - } - } - return {}; -} - QString Ollie9pClient::agentKey(const QString &sessionId, const QString &agentId) const { return sessionId + "\n" + agentId; @@ -159,13 +135,12 @@ QString Ollie9pClient::agentKey(const QString &sessionId, const QString &agentId QList Ollie9pClient::readAgentRecords(const QString &sessionId) { QList result; - QString sessionName = sessionNameForId(sessionId); - if (sessionName.isEmpty()) sessionName = sessionId; - if (sessionName.isEmpty()) return result; - const QString raw = QString::fromUtf8(run9p({"ls", "session/" + sessionName + "/agent"})).trimmed(); + if (sessionId.isEmpty()) return result; + // Use immutable ID — the 9P namespace resolves it via alias. + const QString raw = QString::fromUtf8(run9p({"ls", "session/" + sessionId + "/agent"})).trimmed(); for (const QString &name : raw.split('\n', Qt::SkipEmptyParts)) { if (name == "new") continue; - const QString path = "session/" + sessionName + "/agent/" + name; + const QString path = "session/" + sessionId + "/agent/" + name; const QString id = QString::fromUtf8(run9p({"read", path + "/id"})).trimmed(); AgentRecord r; r.id = id.isEmpty() ? name : id; @@ -239,6 +214,11 @@ void Ollie9pClient::startAgentConnections() reconcileAgentConnections(); } +// reconcileAgentConnections ensures one NinePConnection per known agent. +// Connections use immutable session/agent IDs in their 9P paths. The server +// namespace lists directories by mutable display name, but accepts the +// immutable UUID as an alias during path walks. This means connection paths +// remain valid across session/agent renames without reconnection. void Ollie9pClient::reconcileAgentConnections() { if (!m_daemonConnected) return; @@ -246,7 +226,6 @@ void Ollie9pClient::reconcileAgentConnections() for (const QVariant &value : std::as_const(m_sessions)) { const QVariantMap session = value.toMap(); const QString sid = session.value("id").toString(); - const QString name = session.value("name").toString(); for (const QVariant &agentValue : session.value("agents").toList()) { const QVariantMap agent = agentValue.toMap(); const QString aid = agent.value("id").toString(); @@ -268,8 +247,10 @@ void Ollie9pClient::reconcileAgentConnections() connect(connection, &NinePConnection::disconnected, this, [this, key]() { setAgentConnected(key, false); }); + // Use immutable IDs in path — aliases in the 9P namespace resolve + // these to the correct session/agent regardless of display name. connection->start(ollie9pBin(), {"-a", serverAddr(), "read", "--open-marker", - "session/" + name + "/agent/" + agent.value("name").toString() + "/connection"}, 4000, true); + "session/" + sid + "/agent/" + aid + "/connection"}, 4000, true); } } for (auto it = m_agentConnections.begin(); it != m_agentConnections.end();) { @@ -299,14 +280,8 @@ void Ollie9pClient::refreshSessions() // Don't trim — trailing tabs are significant fields QString raw = QString::fromUtf8(out); - const QVariantList oldSessions = m_sessions; m_sessions.clear(); QSet seen; - QHash sessionNames; - for (const QVariant &value : oldSessions) { - const QVariantMap session = value.toMap(); - sessionNames.insert(session.value("id").toString(), session.value("name").toString()); - } if (!raw.isEmpty()) { for (const QString &line : raw.split('\n', Qt::SkipEmptyParts)) { QStringList parts = line.split('\t'); @@ -316,12 +291,10 @@ void Ollie9pClient::refreshSessions() if (seen.contains(sid)) continue; // already added (multi-agent) seen.insert(sid); QVariantMap session; - // session/idx currently exposes the mutable display name. Read the - // immutable id and retain both values in the GUI model. - QByteArray idOut = run9p({"read", "session/" + sid + "/id"}).trimmed(); - QString sessionId = QString::fromUtf8(idOut); + // session/idx format: name\tstate\tcwd\tbackend\tmodel\tagentName\tid + // The immutable ID is in field 6 (0-indexed), avoiding a round-trip. + QString sessionId = parts.size() > 6 ? parts[6] : ""; if (sessionId.isEmpty()) sessionId = sid; - sessionNames[sessionId] = sid; session["id"] = sessionId; session["name"] = sid; session["state"] = parts.size() > 1 ? parts[1] : ""; @@ -329,9 +302,9 @@ void Ollie9pClient::refreshSessions() session["backend"] = parts.size() > 3 ? parts[3] : ""; session["model"] = parts.size() > 4 ? parts[4] : ""; QVariantList agents; - for (const QString &name : QString::fromUtf8(run9p({"ls", "session/" + sid + "/agent"})).trimmed().split('\n', Qt::SkipEmptyParts)) { + for (const QString &name : QString::fromUtf8(run9p({"ls", "session/" + sessionId + "/agent"})).trimmed().split('\n', Qt::SkipEmptyParts)) { if (name == "new") continue; - const QString path = "session/" + sid + "/agent/" + name; + const QString path = "session/" + sessionId + "/agent/" + name; AgentRecord a; a.id = QString::fromUtf8(run9p({"read", path + "/id"})).trimmed(); a.name = name; @@ -363,11 +336,8 @@ QString Ollie9pClient::readLog() QString Ollie9pClient::readLogForSession(const QString &sessionId, const QString &agentId) { if (sessionId.isEmpty() || agentId.isEmpty()) return {}; - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - QString agentName = agentNameForId(sessionId, agentId); - if (agentName.isEmpty()) agentName = agentId; - QByteArray out = run9p({"read", "session/" + sessName + "/agent/" + agentName + "/log"}); + // Use immutable IDs — the 9P namespace resolves them via aliases. + QByteArray out = run9p({"read", "session/" + sessionId + "/agent/" + agentId + "/log"}); return QString::fromUtf8(out); } @@ -416,11 +386,8 @@ bool Ollie9pClient::killSession(const QString &sessionId) stopStreams(); } - // Resolve the directory name from the immutable session ID - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - - QString path = "session/" + sessName + "/ctl"; + // Use immutable ID — the 9P namespace resolves it via alias. + QString path = "session/" + sessionId + "/ctl"; QProcess proc; proc.start(ninepBin(), {"-a", serverAddr(), "write", path}); proc.waitForStarted(3000); @@ -440,9 +407,8 @@ QString Ollie9pClient::getConfig() QStringList Ollie9pClient::getAgents(const QString &sessionId) { if (sessionId.isEmpty()) return {}; - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - QByteArray out = run9p({"ls", "session/" + sessName + "/agent"}); + // Use immutable ID — the 9P namespace resolves it via alias. + QByteArray out = run9p({"ls", "session/" + sessionId + "/agent"}); QString raw = QString::fromUtf8(out).trimmed(); if (raw.isEmpty()) return {}; QStringList all = raw.split('\n', Qt::SkipEmptyParts); @@ -608,9 +574,6 @@ bool Ollie9pClient::createAgent(const QString &sessionId, const QString &cwd, co { if (sessionId.isEmpty() || cwd.isEmpty()) return false; - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - QStringList agentArgs; agentArgs << "cwd=" + cwd; if (!backend.isEmpty()) agentArgs << "backend=" + backend; @@ -620,7 +583,8 @@ bool Ollie9pClient::createAgent(const QString &sessionId, const QString &cwd, co if (!agentAlias.isEmpty()) agentArgs << "name=" + agentAlias; QProcess proc; - proc.start(ninepBin(), {"-a", serverAddr(), "rdwr", "session/" + sessName + "/agent/new"}); + // Use immutable ID — the 9P namespace resolves it via alias. + proc.start(ninepBin(), {"-a", serverAddr(), "rdwr", "session/" + sessionId + "/agent/new"}); proc.waitForStarted(3000); proc.write((agentArgs.join(" ") + "\n").toUtf8()); proc.closeWriteChannel(); @@ -640,12 +604,8 @@ bool Ollie9pClient::killAgent(const QString &sessionId, const QString &agentId) { if (sessionId.isEmpty() || agentId.isEmpty()) return false; - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - QString agentName = agentNameForId(sessionId, agentId); - if (agentName.isEmpty()) agentName = agentId; - - QString path = "session/" + sessName + "/agent/" + agentName + "/ctl"; + // Use immutable IDs — the 9P namespace resolves them via aliases. + QString path = "session/" + sessionId + "/agent/" + agentId + "/ctl"; QProcess proc; proc.start(ninepBin(), {"-a", serverAddr(), "write", path}); proc.waitForStarted(3000); @@ -665,16 +625,11 @@ bool Ollie9pClient::renameSession(const QString &sessionId, const QString &newNa return false; } - // Session names are mutable display names, not directory IDs. Writing the - // name file also avoids relying on Twstat, which is not implemented by all - // 9P frontends. - // Resolve the directory name from the immutable session ID - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - + // Session names are mutable display names. Writing the name file triggers + // the rename. Use the immutable ID to address the session. QProcess proc; proc.setProgram(bin); - proc.setArguments({"-a", serverAddr(), "write", "session/" + sessName + "/name"}); + proc.setArguments({"-a", serverAddr(), "write", "session/" + sessionId + "/name"}); proc.start(); if (!proc.waitForStarted(3000)) { qDebug() << "renameSession failed to start:" << proc.errorString(); @@ -689,21 +644,9 @@ bool Ollie9pClient::renameSession(const QString &sessionId, const QString &newNa return false; } - // Stop path-based streams before changing the namespace entry. Otherwise - // Guarded streams can observe the old path disappearing and repeatedly - // restart against it while this synchronous write is in progress. - const bool active = m_activeSessionId == sessionId; - const QString activeAgent = m_agentId; - if (active) - stopAgentStreams(); - - if (active) { - m_activeSessionId = newName; - emit activeSessionIdChanged(); - if (!activeAgent.isEmpty()) - switchAgent(newName, activeAgent); - } - + // With immutable-ID paths, renaming no longer invalidates streams or + // the active selection. Just refresh the session list to pick up the + // new display name. refreshSessions(); return true; } @@ -718,23 +661,12 @@ bool Ollie9pClient::renameAgent(const QString &sessionId, const QString &agentId return false; } - // Agent names are mutable display names. Write the name file directly; - // ctl accepts commands, not name assignments, and the agent directory is - // keyed by its current display name. - const bool active = m_activeSessionId == sessionId && m_agentId == agentId; - if (active) - stopAgentStreams(); - - // Resolve directory names from immutable IDs - QString sessName = sessionNameForId(sessionId); - if (sessName.isEmpty()) sessName = sessionId; - QString agentName = agentNameForId(sessionId, agentId); - if (agentName.isEmpty()) agentName = agentId; - + // Agent names are mutable display names. Write the name file directly + // using immutable IDs to address the path. QProcess proc; proc.setProgram(bin); proc.setArguments({"-a", serverAddr(), "write", - "session/" + sessName + "/agent/" + agentName + "/name"}); + "session/" + sessionId + "/agent/" + agentId + "/name"}); proc.start(); if (!proc.waitForStarted(3000)) { qDebug() << "renameAgent failed to start:" << proc.errorString(); @@ -749,11 +681,7 @@ bool Ollie9pClient::renameAgent(const QString &sessionId, const QString &agentId return false; } - if (active) { - // switchAgent updates the active ID and restarts the path-based streams. - switchAgent(sessionId, newName); - } - + // With immutable-ID paths, renaming no longer invalidates streams. refreshSessions(); return true; } @@ -815,10 +743,8 @@ void Ollie9pClient::stopStreams() QString Ollie9pClient::agentPath() const { - const QString sessionName = sessionNameForId(m_activeSessionId); - const QString agentName = agentNameForId(m_activeSessionId, m_agentId); - return "session/" + (sessionName.isEmpty() ? m_activeSessionId : sessionName) - + "/agent/" + (agentName.isEmpty() ? m_agentId : agentName); + // Use immutable IDs — the 9P namespace resolves them via aliases. + return "session/" + m_activeSessionId + "/agent/" + m_agentId; } QByteArray Ollie9pClient::run9p(const QStringList &args) diff --git a/gui/ollie9pclient.h b/gui/ollie9pclient.h index aa7e6d6..5bde44f 100644 --- a/gui/ollie9pclient.h +++ b/gui/ollie9pclient.h @@ -111,8 +111,6 @@ private: void stopAgentStreams(); void stopStreams(); QString agentPath() const; - QString sessionNameForId(const QString &sessionId) const; - QString agentNameForId(const QString &sessionId, const QString &agentId) const; QByteArray run9p(const QStringList &args); void ensureRootDataLoaded(); void setDaemonConnected(bool connected);