From dc3100f054448f61177b2403af16460905118daf Mon Sep 17 00:00:00 2001 From: Petr Mironychev <9195189+Palm1r@users.noreply.github.com> Date: Sun, 28 Jun 2026 17:37:11 +0200 Subject: [PATCH] fix: Remove isUserSource from tests --- ChatView/ChatRootView.cpp | 36 +++++---- ChatView/ChatRootView.hpp | 2 + ChatView/ClientInterface.cpp | 120 +++++++++++++++++------------ ChatView/ClientInterface.hpp | 7 ++ ChatView/SessionFileRegistry.cpp | 30 ++++++-- ChatView/SessionFileRegistry.hpp | 19 +++-- sources/Session/Session.cpp | 46 +++++++++-- sources/Session/Session.hpp | 3 + sources/Session/SessionManager.cpp | 37 +++++++++ sources/Session/SessionManager.hpp | 4 + test/AgentLoaderTest.cpp | 2 +- test/DocumentContextReaderTest.cpp | 6 +- 12 files changed, 224 insertions(+), 88 deletions(-) diff --git a/ChatView/ChatRootView.cpp b/ChatView/ChatRootView.cpp index 67ff711..6a22701 100644 --- a/ChatView/ChatRootView.cpp +++ b/ChatView/ChatRootView.cpp @@ -357,6 +357,15 @@ SessionManager *ChatRootView::sessionManager() const return m_sessionManager; } +QodeAssist::Session *ChatRootView::ownerSession() +{ + if (!m_clientInterface) + return nullptr; + m_clientInterface->setSessionManager(sessionManager()); + m_clientInterface->ensureSession(); + return m_clientInterface->session(); +} + void ChatRootView::loadAvailableChatAgents() { m_agentController->setAgentFactory(agentFactory()); @@ -535,13 +544,12 @@ void ChatRootView::clearMessages() void ChatRootView::saveHistory(const QString &filePath) { - if (filePath != m_recentFilePath) { - if (auto registry = sessionFileRegistry(); registry && registry->isLocked(filePath)) { - m_lastErrorMessage - = tr("This chat file is already in use by another QodeAssist chat session."); - emit lastErrorMessageChanged(); - return; - } + if (auto registry = sessionFileRegistry(); + registry && registry->isLockedByOther(filePath, ownerSession())) { + m_lastErrorMessage = tr( + "This chat file is already in use by another QodeAssist chat session."); + emit lastErrorMessageChanged(); + return; } auto result = m_historyStore->save(filePath); @@ -554,13 +562,11 @@ void ChatRootView::saveHistory(const QString &filePath) void ChatRootView::loadHistory(const QString &filePath) { - if (filePath != m_recentFilePath) { - if (auto registry = sessionFileRegistry(); registry && registry->isLocked(filePath)) { - m_lastErrorMessage - = tr("This chat is already open in another QodeAssist chat session."); - emit lastErrorMessageChanged(); - return; - } + if (auto registry = sessionFileRegistry(); + registry && registry->isLockedByOther(filePath, ownerSession())) { + m_lastErrorMessage = tr("This chat is already open in another QodeAssist chat session."); + emit lastErrorMessageChanged(); + return; } auto result = m_historyStore->load(filePath); @@ -1032,7 +1038,7 @@ void ChatRootView::setRecentFilePath(const QString &filePath) registry->release(m_recentFilePath); } if (!filePath.isEmpty()) { - registry->lock(filePath); + registry->lock(filePath, ownerSession()); } } diff --git a/ChatView/ChatRootView.hpp b/ChatView/ChatRootView.hpp index cc68f09..3175027 100644 --- a/ChatView/ChatRootView.hpp +++ b/ChatView/ChatRootView.hpp @@ -21,6 +21,7 @@ namespace QodeAssist { class AgentFactory; class SessionManager; class ConversationHistory; +class Session; } namespace QodeAssist::Chat { @@ -233,6 +234,7 @@ private: Skills::SkillsManager *skillsManager() const; AgentFactory *agentFactory() const; SessionManager *sessionManager() const; + QodeAssist::Session *ownerSession(); QodeAssist::ConversationHistory *m_history; ChatModel *m_chatModel; diff --git a/ChatView/ClientInterface.cpp b/ChatView/ClientInterface.cpp index ae732e9..a14e12c 100644 --- a/ChatView/ClientInterface.cpp +++ b/ChatView/ClientInterface.cpp @@ -97,6 +97,55 @@ void ClientInterface::setActiveAgent(const QString &agentName) m_activeAgent = agentName; } +Session *ClientInterface::session() const +{ + return m_session; +} + +void ClientInterface::ensureSession() +{ + if (m_session || !m_sessionManager || !m_history) + return; + + m_session = m_sessionManager->createDetachedSession(m_history, this); + + connect(m_session, &Session::event, this, [this](const QodeAssist::ResponseEvent &ev) { + onSessionEvent(m_session, ev); + }); + connect(m_session, &Session::finished, this, [this](const LLMQore::RequestID &id, const QString &) { + onSessionFinished(id); + }); + connect( + m_session, + &Session::failed, + this, + [this](const LLMQore::RequestID &id, const QodeAssist::ErrorInfo &error) { + onSessionFailed(id, error); + }); +} + +bool ClientInterface::ensureAgentBound() +{ + if (m_session->hasAgent() && m_boundAgent == m_activeAgent) + return true; + + QString agentError; + if (!m_sessionManager->rebindAgentByName(m_session, m_activeAgent, &agentError)) { + m_boundAgent.clear(); + const QString error = agentError.isEmpty() ? QStringLiteral("No chat agent selected") + : agentError; + LOG_MESSAGE(error); + emit errorOccurred(error); + return false; + } + m_boundAgent = m_activeAgent; + + if (auto *client = m_session->client()) + m_sessionManager->toolContributors().contribute(client->tools()); + + return true; +} + void ClientInterface::sendMessage( const QString &message, const QList &attachments, @@ -173,22 +222,21 @@ void ClientInterface::sendMessage( return; } - QString sessionError; - Session *session = m_sessionManager->createSession(m_activeAgent, m_history, &sessionError); - if (!session) { - const QString error = sessionError.isEmpty() - ? QStringLiteral("No chat agent selected") - : sessionError; + ensureSession(); + if (!m_session) { + const QString error = QStringLiteral("Failed to create chat session"); LOG_MESSAGE(error); emit errorOccurred(error); return; } - auto *client = session->client(); + if (!ensureAgentBound()) + return; + + auto *client = m_session->client(); if (!client) { const QString error = QStringLiteral("Chat agent has no live client"); LOG_MESSAGE(error); - m_sessionManager->removeSession(session); emit errorOccurred(error); return; } @@ -197,26 +245,27 @@ void ClientInterface::sendMessage( Templates::ContextRenderer::Bindings bindings; bindings.projectDir = project ? project->projectDirectory().toFSPathString() : QString(); bindings.configDir = AgentFactory::userConfigDir(); - session->setContextBindings(bindings); + m_session->setContextBindings(bindings); const QString chatFilePath = m_chatFilePath; - session->setContentLoader([chatFilePath, cache = m_contentCache](const QString &storedPath) { + m_session->setContentLoader([chatFilePath, cache = m_contentCache](const QString &storedPath) { return ChatSerializer::loadContentFromStorage(chatFilePath, storedPath, cache.get()); }); - m_sessionManager->toolContributors().contribute(client->tools()); client->toolLoop()->setMaxRounds(Settings::toolsSettings().maxToolContinuations()); client->setTransferTimeout( static_cast(Settings::generalSettings().requestTimeout() * 1000)); const QString chatContext = buildChatContextLayer(); - if (!chatContext.isEmpty()) - session->systemPrompt()->setLayer(QStringLiteral("chat.context"), chatContext); + if (chatContext.isEmpty()) + m_session->systemPrompt()->clearLayer(QStringLiteral("chat.context")); + else + m_session->systemPrompt()->setLayer(QStringLiteral("chat.context"), chatContext); if (linkedFiles.isEmpty()) { - session->unpinContext(QStringLiteral("chat.files")); + m_session->unpinContext(QStringLiteral("chat.files")); } else { - session->pinContext( + m_session->pinContext( QStringLiteral("chat.files"), [contextManager = QPointer(m_contextManager), linkedFiles]() -> QString { @@ -265,29 +314,16 @@ void ClientInterface::sendMessage( } } - connect(session, &Session::event, this, [this, session](const QodeAssist::ResponseEvent &ev) { - onSessionEvent(session, ev); - }); - connect( - session, &Session::finished, this, - [this](const LLMQore::RequestID &id, const QString &) { onSessionFinished(id); }); - connect( - session, &Session::failed, this, - [this](const LLMQore::RequestID &id, const QodeAssist::ErrorInfo &error) { - onSessionFailed(id, error); - }); - - const LLMQore::RequestID requestId = session->send(std::move(blocks)); + const LLMQore::RequestID requestId = m_session->send(std::move(blocks)); if (requestId.isEmpty()) { const QString error = QStringLiteral("Failed to start chat request for agent '%1': %2") - .arg(m_activeAgent, session->lastError().message); + .arg(m_activeAgent, m_session->lastError().message); LOG_MESSAGE(error); - m_sessionManager->removeSession(session); emit errorOccurred(error); return; } - m_activeRequests[requestId] = {QJsonObject{{"id", requestId}}, session}; + m_activeRequests[requestId] = {QJsonObject{{"id", requestId}}, m_session}; emit requestStarted(requestId); } @@ -337,8 +373,6 @@ void ClientInterface::onSessionFinished(const QString &requestId) if (it == m_activeRequests.end()) return; - Session *session = it.value().session; - QString applyError; if (!Context::ChangesManager::instance().applyPendingEditsForRequest(requestId, &applyError)) { LOG_MESSAGE(QString("Some edits for request %1 were not auto-applied: %2") @@ -348,9 +382,6 @@ void ClientInterface::onSessionFinished(const QString &requestId) emit messageReceivedCompletely(); m_activeRequests.erase(it); - - if (session && m_sessionManager) - m_sessionManager->removeSession(session); } void ClientInterface::onSessionFailed(const QString &requestId, const QodeAssist::ErrorInfo &error) @@ -359,15 +390,10 @@ void ClientInterface::onSessionFailed(const QString &requestId, const QodeAssist if (it == m_activeRequests.end()) return; - Session *session = it.value().session; - LOG_MESSAGE(QString("Chat request %1 failed: %2").arg(requestId, error.message)); emit errorOccurred(error.message); m_activeRequests.erase(it); - - if (session && m_sessionManager) - m_sessionManager->removeSession(session); } QStringList ClientInterface::invokedSkillNames(const QString &message) const @@ -419,20 +445,18 @@ QString ClientInterface::buildChatContextLayer() const void ClientInterface::clearMessages() { + if (m_session) + m_session->cancel(); + m_activeRequests.clear(); if (m_history) m_history->clear(); } void ClientInterface::cancelRequest() { - const auto requests = m_activeRequests; m_activeRequests.clear(); - - for (auto it = requests.begin(); it != requests.end(); ++it) { - Session *session = it.value().session; - if (session && m_sessionManager) - m_sessionManager->removeSession(session); - } + if (m_session) + m_session->cancel(); LOG_MESSAGE("All chat requests cancelled and state cleared"); } diff --git a/ChatView/ClientInterface.hpp b/ChatView/ClientInterface.hpp index e11c465..3110fda 100644 --- a/ChatView/ClientInterface.hpp +++ b/ChatView/ClientInterface.hpp @@ -55,6 +55,9 @@ public: void setChatFilePath(const QString &filePath); QString chatFilePath() const; + void ensureSession(); + Session *session() const; + signals: void errorOccurred(const QString &error); void messageReceivedCompletely(); @@ -63,6 +66,8 @@ signals: int promptTokens, int completionTokens, int cachedPromptTokens, int reasoningTokens); private: + bool ensureAgentBound(); + void onSessionEvent(Session *session, const QodeAssist::ResponseEvent &ev); void onSessionFinished(const QString &requestId); void onSessionFailed(const QString &requestId, const QodeAssist::ErrorInfo &error); @@ -85,7 +90,9 @@ private: QPointer m_history; Skills::SkillsManager *m_skillsManager = nullptr; QPointer m_sessionManager; + QPointer m_session; QString m_activeAgent; + QString m_boundAgent; QString m_chatFilePath; std::shared_ptr m_contentCache; diff --git a/ChatView/SessionFileRegistry.cpp b/ChatView/SessionFileRegistry.cpp index 150878d..9527d4a 100644 --- a/ChatView/SessionFileRegistry.cpp +++ b/ChatView/SessionFileRegistry.cpp @@ -8,29 +8,43 @@ #include +#include + namespace QodeAssist::Chat { SessionFileRegistry::SessionFileRegistry(QObject *parent) : QObject(parent) {} +SessionFileRegistry::~SessionFileRegistry() = default; + bool SessionFileRegistry::isLocked(const QString &path) const { - return !path.isEmpty() && m_lockedPaths.contains(path); + return !path.isEmpty() && !m_locks.value(path).isNull(); } -bool SessionFileRegistry::lock(const QString &path) +bool SessionFileRegistry::isLockedByOther(const QString &path, QodeAssist::Session *self) const { - if (path.isEmpty() || m_lockedPaths.contains(path)) { + if (path.isEmpty()) return false; - } - m_lockedPaths.insert(path); + const auto owner = m_locks.value(path); + return !owner.isNull() && owner != self; +} + +bool SessionFileRegistry::lock(const QString &path, QodeAssist::Session *owner) +{ + if (path.isEmpty()) + return false; + const auto existing = m_locks.value(path); + if (!existing.isNull() && existing != owner) + return false; + m_locks.insert(path, owner); return true; } void SessionFileRegistry::release(const QString &path) { - m_lockedPaths.remove(path); + m_locks.remove(path); } void SessionFileRegistry::setPendingChatFile(const QString &path) @@ -45,7 +59,7 @@ QString SessionFileRegistry::takePendingChatFile() QString SessionFileRegistry::uniqueFreePath(const QString &desiredPath) const { - if (desiredPath.isEmpty() || !m_lockedPaths.contains(desiredPath)) { + if (desiredPath.isEmpty() || !isLocked(desiredPath)) { return desiredPath; } @@ -59,7 +73,7 @@ QString SessionFileRegistry::uniqueFreePath(const QString &desiredPath) const if (!suffix.isEmpty()) { candidate += '.' + suffix; } - if (!m_lockedPaths.contains(candidate)) { + if (!isLocked(candidate)) { return candidate; } } diff --git a/ChatView/SessionFileRegistry.hpp b/ChatView/SessionFileRegistry.hpp index 1b06649..ca0d8de 100644 --- a/ChatView/SessionFileRegistry.hpp +++ b/ChatView/SessionFileRegistry.hpp @@ -4,24 +4,31 @@ #pragma once +#include #include -#include +#include #include +namespace QodeAssist { +class Session; +} + namespace QodeAssist::Chat { -// Shared registry of chat session (autosave) file paths that are currently held by a live -// chat instance. Lets every chat view — bottom pane, navigation panel, editor split — claim -// a unique history file so two sessions never autosave into the same path. +// Shared registry mapping each chat (autosave) file to the live Session that owns it, so a +// file is busy only while its owning Session is alive (a destroyed Session frees it — the +// QPointer goes null). Keeps two chat views from autosaving into the same path. class SessionFileRegistry : public QObject { Q_OBJECT public: explicit SessionFileRegistry(QObject *parent = nullptr); + ~SessionFileRegistry() override; bool isLocked(const QString &path) const; - bool lock(const QString &path); + bool isLockedByOther(const QString &path, QodeAssist::Session *self) const; + bool lock(const QString &path, QodeAssist::Session *owner); void release(const QString &path); QString uniqueFreePath(const QString &desiredPath) const; @@ -32,7 +39,7 @@ public: QString takePendingChatFile(); private: - QSet m_lockedPaths; + QHash> m_locks; QString m_pendingChatFile; }; diff --git a/sources/Session/Session.cpp b/sources/Session/Session.cpp index 197199f..fcb39e1 100644 --- a/sources/Session/Session.cpp +++ b/sources/Session/Session.cpp @@ -33,14 +33,33 @@ Session::Session(Agent *agent, QObject *parent) Session::Session(Agent *agent, ConversationHistory *externalHistory, QObject *parent) : QObject(parent) - , m_agent(agent) , m_history(externalHistory ? externalHistory : new ConversationHistory(this)) , m_systemPrompt(new SystemPromptBuilder(this)) { - if (!m_agent) { - m_invalidReason = QStringLiteral("Session: agent is null"); + if (agent) + setAgent(agent); +} + +void Session::setAgent(Agent *agent) +{ + if (agent == m_agent) return; + + if (isInFlight()) + teardownInFlight(); + + if (m_router) { + delete m_router; + m_router = nullptr; } + + delete m_agent; + m_agent = agent; + m_invalidReason.clear(); + + if (!m_agent) + return; + m_agent->setParent(this); if (!m_agent->isValid()) { @@ -55,8 +74,7 @@ Session::Session(Agent *agent, ConversationHistory *externalHistory, QObject *pa return; } if (!m_agent->promptTemplate()) { - m_invalidReason - = QStringLiteral("Session: agent has no inline prompt template"); + m_invalidReason = QStringLiteral("Session: agent has no inline prompt template"); return; } @@ -85,6 +103,16 @@ bool Session::isInFlight() const noexcept return !m_inFlight.isEmpty(); } +bool Session::hasAgent() const noexcept +{ + return m_agent != nullptr; +} + +bool Session::canSend() const noexcept +{ + return isValid() && m_agent != nullptr && client() != nullptr; +} + const ErrorInfo &Session::lastError() const noexcept { return m_lastError; @@ -128,8 +156,12 @@ void Session::unpinContext(const QString &id) LLMQore::RequestID Session::send(std::vector> userBlocks) { - if (!isValid()) { - m_lastError = makeError(ErrorCategory::Config, invalidReason()); + if (!canSend()) { + const QString reason = m_agent ? (invalidReason().isEmpty() + ? QStringLiteral("Session: agent has no live client") + : invalidReason()) + : QStringLiteral("Session: no agent bound"); + m_lastError = makeError(ErrorCategory::Config, reason); return {}; } if (userBlocks.empty() || !m_history) { diff --git a/sources/Session/Session.hpp b/sources/Session/Session.hpp index 3c09c56..f95949c 100644 --- a/sources/Session/Session.hpp +++ b/sources/Session/Session.hpp @@ -46,6 +46,8 @@ public: bool isValid() const noexcept; QString invalidReason() const; bool isInFlight() const noexcept; + bool hasAgent() const noexcept; + bool canSend() const noexcept; const ErrorInfo &lastError() const noexcept; using ContentLoader = ContextAssembler::ContentLoader; @@ -56,6 +58,7 @@ public: void unpinContext(const QString &id); Agent *agent() noexcept { return m_agent; } + void setAgent(Agent *agent); ConversationHistory *history() const noexcept { return m_history; } SystemPromptBuilder *systemPrompt() const noexcept { return m_systemPrompt; } diff --git a/sources/Session/SessionManager.cpp b/sources/Session/SessionManager.cpp index a913ecb..7b61810 100644 --- a/sources/Session/SessionManager.cpp +++ b/sources/Session/SessionManager.cpp @@ -59,6 +59,43 @@ Session *SessionManager::createSession( return session; } +Session *SessionManager::createDetachedSession(ConversationHistory *externalHistory, QObject *parent) +{ + return new Session(/*agent=*/nullptr, externalHistory, parent); +} + +bool SessionManager::rebindAgentByName(Session *session, const QString &agentName, QString *errorOut) +{ + if (!session) { + if (errorOut) + *errorOut = QStringLiteral("SessionManager: null session"); + return false; + } + if (!m_agentFactory) { + if (errorOut) + *errorOut = QStringLiteral("SessionManager: no AgentFactory bound"); + return false; + } + + QString agentErr; + Agent *agent = m_agentFactory->create(agentName, /*parent=*/nullptr, &agentErr); + if (!agent) { + if (errorOut) + *errorOut = agentErr.isEmpty() + ? QStringLiteral("SessionManager: agent '%1' not found").arg(agentName) + : agentErr; + return false; + } + + session->setAgent(agent); + if (!session->isValid()) { + if (errorOut) + *errorOut = session->invalidReason(); + return false; + } + return true; +} + Session *SessionManager::acquire(const QString &agentName, QString *errorOut) { auto &bucket = m_pool[agentName]; diff --git a/sources/Session/SessionManager.hpp b/sources/Session/SessionManager.hpp index 0e19e73..f6842c5 100644 --- a/sources/Session/SessionManager.hpp +++ b/sources/Session/SessionManager.hpp @@ -34,6 +34,10 @@ public: ConversationHistory *externalHistory, QString *errorOut = nullptr); + Session *createDetachedSession(ConversationHistory *externalHistory, QObject *parent); + + bool rebindAgentByName(Session *session, const QString &agentName, QString *errorOut = nullptr); + Session *acquire(const QString &agentName, QString *errorOut = nullptr); void release(Session *session); diff --git a/test/AgentLoaderTest.cpp b/test/AgentLoaderTest.cpp index 59f4bb5..158da9b 100644 --- a/test/AgentLoaderTest.cpp +++ b/test/AgentLoaderTest.cpp @@ -98,7 +98,7 @@ TEST(AgentLoaderTest, UserAgentCollidingWithBundledNameIsRejected) const AgentConfig *cfg = findConfig(result, QStringLiteral("A")); ASSERT_NE(cfg, nullptr); EXPECT_EQ(cfg->description, QStringLiteral("base")); - EXPECT_FALSE(cfg->isUserSource()); + EXPECT_TRUE(cfg->sourcePath.startsWith(bundled.path())); } TEST(AgentLoaderTest, HiddenIsNotInherited) diff --git a/test/DocumentContextReaderTest.cpp b/test/DocumentContextReaderTest.cpp index 7d53cff..3fcbd91 100644 --- a/test/DocumentContextReaderTest.cpp +++ b/test/DocumentContextReaderTest.cpp @@ -370,7 +370,7 @@ TEST_F(DocumentContextReaderTest, testPrepareContext) (QodeAssist::Templates::ContextData{ .prefix = "Line 0\nLine 1\nLin", .suffix = "e 2\nLine 3\nLine 4", - .fileContext = "\n Language: (MIME: text/python) filepath: /path/to/file()\n\n" + .fileContext = "\nFile information:\nMIME type: text/python\nFile path: /path/to/file\n\n" "Recent Project Changes Context:\n "})); EXPECT_EQ( @@ -378,7 +378,7 @@ TEST_F(DocumentContextReaderTest, testPrepareContext) (QodeAssist::Templates::ContextData{ .prefix = "Line 1\nLin", .suffix = "e 2\nLine 3", - .fileContext = "\n Language: (MIME: text/python) filepath: /path/to/file()\n\n" + .fileContext = "\nFile information:\nMIME type: text/python\nFile path: /path/to/file\n\n" "Recent Project Changes Context:\n "})); EXPECT_EQ( @@ -386,7 +386,7 @@ TEST_F(DocumentContextReaderTest, testPrepareContext) (QodeAssist::Templates::ContextData{ .prefix = "Line 0\nLine 1\nLin", .suffix = "e 2\nLine 3\nLine 4", - .fileContext = "\n Language: (MIME: text/python) filepath: /path/to/file()\n\n" + .fileContext = "\nFile information:\nMIME type: text/python\nFile path: /path/to/file\n\n" "Recent Project Changes Context:\n "})); }