diff --git a/.agent/plans/acp-improvement.md b/.agent/plans/acp-improvement.md new file mode 100644 index 000000000..ef606d089 --- /dev/null +++ b/.agent/plans/acp-improvement.md @@ -0,0 +1,54 @@ +# Plan: ACP Support Improvement for ecode + +This plan outlines the steps to complete and improve the Agent Client Protocol (ACP) implementation in `ecode`. + +## Current Status & Gaps +The current implementation has the basic structure but misses several critical features for a full ACP experience: +- **Terminal output** is not captured (returns empty). +- **Tool call updates** are not handled in the UI. +- **Plan schema** is non-standard. +- **Slash commands** are not exposed to the user. +- **Terminal limits** are ignored. + +## Proposed Steps + +### 1. Research & Refinement (Done) +- [x] Analyze ACP documentation. +- [x] Review current `ecode` implementation. +- [x] Identify missing pieces. + +### 2. Phase 1: Terminal Output Capture (Done) +We need a way to capture the output from the terminal process to satisfy `terminal/output` requests. +- [x] Modify `src/modules/eterm/include/eterm/terminal/terminalemulator.hpp`: + - Add `using DataCb = std::function;` + - Add `void setDataCb(DataCb cb);` +- [x] Modify `src/modules/eterm/src/eterm/terminal/terminalemulator.cpp`: + - Call `mDataCb` in `ttyread()` when new data is received. +- [x] Update `src/tools/ecode/plugins/aiassistant/acp/agentsession.hpp`: + - Add an output buffer to `TermData`. +- [x] Update `src/tools/ecode/plugins/aiassistant/acp/agentsession.cpp`: + - Set the `DataCb` in `onTerminalCreated`. + - Implement `onTerminalOutput` to return and clear the buffer. + - Respect `outputByteLimit`. + +### 3. Phase 2: UI & Protocol Updates +- [ ] Update `src/tools/ecode/plugins/aiassistant/chatui.cpp`: + - Fix `plan` update handling to use the standard `entries` schema. + - Implement `tool_call_update` handling. + - Update tool call UI to reflect status (pending, in_progress, completed, failed, cancelled). +- [ ] Implement embedded terminals in tool calls: + - When a `tool_call_update` contains a `terminalId`, link the terminal UI to that tool call bubble if possible. + +### 4. Phase 3: UX Improvements +- [ ] Expose slash commands: + - Show `mAvailableCommands` in the UI. + - Add simple autocompletion for `/` commands in `mChatInput`. +- [ ] Enhance terminal management: + - Ensure `terminal/wait_for_exit` works correctly (async). + +### 5. Phase 4: Validation +- [ ] Test with a compatible ACP agent. +- [ ] Verify file system tools. +- [ ] Verify terminal creation and output reading. +- [ ] Verify plan updates. +- [ ] Verify tool call permissions. diff --git a/include/eepp/graphics/fonttruetype.hpp b/include/eepp/graphics/fonttruetype.hpp index 6db1b9807..d75dd6d8d 100644 --- a/include/eepp/graphics/fonttruetype.hpp +++ b/include/eepp/graphics/fonttruetype.hpp @@ -67,7 +67,8 @@ class EE_API FontTrueType : public Font { bool loaded() const; - FontTrueType& operator=( const FontTrueType& right ); + FontTrueType( const FontTrueType& ) = delete; + FontTrueType& operator=( const FontTrueType& ) = delete; bool getBoldAdvanceSameAsRegular() const; @@ -182,11 +183,11 @@ class EE_API FontTrueType : public Font { GlyphTable glyphs; ///< Table mapping code points to their corresponding glyph GlyphDrawableTable - drawables; ///> Table mapping code points to their corresponding glyph drawables. - Texture* texture; ///< Texture containing the pixels of the glyphs - std::vector rows; ///< List containing the position of all the existing rows - Uint32 fontInternalId{ 0 }; // The font internal id - unsigned int nextRow; ///< Y position of the next new row in the texture + drawables; ///> Table mapping code points to their corresponding glyph drawables. + Texture* texture{ nullptr }; ///< Texture containing the pixels of the glyphs + std::vector rows; ///< List containing the position of all the existing rows + Uint32 fontInternalId{ 0 }; // The font internal id + unsigned int nextRow; ///< Y position of the next new row in the texture const FontTrueType* font{ nullptr }; }; @@ -212,14 +213,14 @@ class EE_API FontTrueType : public Font { typedef UnorderedMap> PageTable; ///< Table mapping a character size to its page (texture) - void* mLibrary; ///< Pointer to the internal library interface (it is typeless to avoid exposing - ///< implementation details) - void* mFace; ///< Pointer to the internal font face (it is typeless to avoid exposing - ///< implementation details) - void* mStreamRec; ///< Pointer to the stream rec instance (it is typeless to avoid exposing - ///< implementation details) - void* mStroker; ///< Pointer to the stroker (it is typeless to avoid exposing implementation - ///< details) + void* mLibrary{ nullptr }; ///< Pointer to the internal library interface (it is typeless to + ///< avoid exposing implementation details) + void* mFace{ nullptr }; ///< Pointer to the internal font face (it is typeless to avoid exposing + ///< implementation details) + void* mStreamRec{ nullptr }; ///< Pointer to the stream rec instance (it is typeless to avoid + ///< exposing implementation details) + void* mStroker{ nullptr }; ///< Pointer to the stroker (it is typeless to avoid exposing + ///< implementation details) void* mHBFont{ nullptr }; mutable ScopedBuffer mMemCopy; ///< If loaded from memory, this is the file copy in memory Font::Info mInfo; ///< Information about the font diff --git a/include/eepp/system/resourcemanager.hpp b/include/eepp/system/resourcemanager.hpp index b282c1684..43dd7b639 100644 --- a/include/eepp/system/resourcemanager.hpp +++ b/include/eepp/system/resourcemanager.hpp @@ -316,7 +316,15 @@ template T* ResourceManagerMulti::add( T* resource ) { template bool ResourceManagerMulti::remove( T* resource, bool remove ) { if ( NULL != resource ) { - mResources.erase( resource->getId() ); + auto range = mResources.equal_range( resource->getId() ); + auto it = range.first; + while ( it != range.second ) { + if ( it->second == resource ) { + mResources.erase( it ); + break; + } + it++; + } if ( remove ) eeSAFE_DELETE( resource ); diff --git a/src/eepp/graphics/fontmanager.cpp b/src/eepp/graphics/fontmanager.cpp index e29e718a1..dd5e9dda6 100644 --- a/src/eepp/graphics/fontmanager.cpp +++ b/src/eepp/graphics/fontmanager.cpp @@ -7,7 +7,11 @@ SINGLETON_DECLARE_IMPLEMENTATION( FontManager ) FontManager::FontManager() {} -FontManager::~FontManager() {} +FontManager::~FontManager() { + mEmojiFont = nullptr; + mColorEmojiFont = nullptr; + mFallbackFonts.clear(); +} Graphics::Font* FontManager::add( Graphics::Font* font ) { eeASSERT( NULL != font ); diff --git a/src/eepp/graphics/fonttruetype.cpp b/src/eepp/graphics/fonttruetype.cpp index f151bc721..559f2809d 100644 --- a/src/eepp/graphics/fonttruetype.cpp +++ b/src/eepp/graphics/fonttruetype.cpp @@ -251,10 +251,31 @@ FontTrueType::FontTrueType( const std::string& FontName ) : mFace( NULL ), mStreamRec( NULL ), mStroker( NULL ), - mInfo(), + mHBFont( NULL ), + mFontInternalId( 0 ), mBoldAdvanceSameAsRegular( false ), + mIsColorEmojiFont( false ), + mIsEmojiFont( false ), + mHasSvgGlyphs( false ), + mHasColrGlyphs( false ), + mIsBitmapOnly( false ), + mIsMonospace( false ), + mIsMonospaceComplete( false ), + mUsingFallback( false ), + mEnableEmojiFallback( true ), + mEnableFallbackFont( true ), + mEnableDynamicMonospace( false ), + mIsBold( false ), + mIsItalic( false ), + mIsMonospaceCompletePending( false ), mHinting( FontManager::instance()->getHinting() ), - mAntialiasing( FontManager::instance()->getAntialiasing() ) {} + mAntialiasing( FontManager::instance()->getAntialiasing() ), + mFontBold( nullptr ), + mFontItalic( nullptr ), + mFontBoldItalic( nullptr ), + mFontBoldCb( 0 ), + mFontItalicCb( 0 ), + mFontBoldItalicCb( 0 ) {} FontTrueType::~FontTrueType() { cleanup(); @@ -965,20 +986,6 @@ bool FontTrueType::loaded() const { return NULL != mFace; } -FontTrueType& FontTrueType::operator=( const FontTrueType& right ) { - FontTrueType temp( right.getName() ); - - temp.mMemCopy.swap( right.mMemCopy ); - std::swap( mLibrary, temp.mLibrary ); - std::swap( mFace, temp.mFace ); - std::swap( mStreamRec, temp.mStreamRec ); - std::swap( mStroker, temp.mStroker ); - std::swap( mInfo, temp.mInfo ); - std::swap( mPages, temp.mPages ); - std::swap( mPixelBuffer, temp.mPixelBuffer ); - return *this; -} - void FontTrueType::cleanup() { sendEvent( Event::Unload ); @@ -1028,9 +1035,36 @@ void FontTrueType::cleanup() { mLibrary = NULL; mFace = NULL; mStroker = NULL; + mHBFont = NULL; mStreamRec = NULL; + mInfo = Info(); + mFontInternalId = 0; + mBoldAdvanceSameAsRegular = false; + mIsColorEmojiFont = false; + mIsEmojiFont = false; + mHasSvgGlyphs = false; + mHasColrGlyphs = false; + mIsBitmapOnly = false; + mIsMonospace = false; + mIsMonospaceComplete = false; + mUsingFallback = false; + mEnableEmojiFallback = true; + mEnableFallbackFont = true; + mEnableDynamicMonospace = false; + mIsBold = false; + mIsItalic = false; + mIsMonospaceCompletePending = false; + mFontBold = nullptr; + mFontItalic = nullptr; + mFontBoldItalic = nullptr; + mFontBoldCb = 0; + mFontItalicCb = 0; + mFontBoldItalicCb = 0; mPages.clear(); std::vector().swap( mPixelBuffer ); + mCodePointIndexCache.clear(); + mKeyCache.clear(); + mClosestCharacterSize.clear(); } static int fontSetLoadOptions( FontAntialiasing antialiasing, FontHinting hinting ) { @@ -1430,6 +1464,10 @@ bool FontTrueType::setCurrentSize( unsigned int characterSize ) const { // only when necessary to avoid killing performances FT_Face face = static_cast( mFace ); + + if ( !face || !face->size ) + return false; + FT_UShort currentSize = face->size->metrics.x_ppem; if ( currentSize != characterSize ) { diff --git a/src/modules/eterm/include/eterm/terminal/terminalemulator.hpp b/src/modules/eterm/include/eterm/terminal/terminalemulator.hpp index 1e1cbc15e..1d43c28aa 100644 --- a/src/modules/eterm/include/eterm/terminal/terminalemulator.hpp +++ b/src/modules/eterm/include/eterm/terminal/terminalemulator.hpp @@ -254,6 +254,10 @@ class TerminalEmulator final { mPromptStateChangedCb = promptStateChangedCb; } + using DataCb = std::function; + + void setDataCb( DataCb cb ) { mDataCb = cb; } + int getTerminalMode() const { return mTerm.mode; } private: @@ -291,6 +295,7 @@ class TerminalEmulator final { std::string mCurrentWorkingDirectory; PromptState mPromptState{ PromptState::Unknown }; PromptStateChangedCb mPromptStateChangedCb; + DataCb mDataCb; void setClipboard( const char* str ); diff --git a/src/modules/eterm/src/eterm/terminal/terminalemulator.cpp b/src/modules/eterm/src/eterm/terminal/terminalemulator.cpp index 3ba018492..fe633c99e 100644 --- a/src/modules/eterm/src/eterm/terminal/terminalemulator.cpp +++ b/src/modules/eterm/src/eterm/terminal/terminalemulator.cpp @@ -653,6 +653,9 @@ size_t TerminalEmulator::ttyread( void ) { _die( "couldn't read from shell: %s\n", strerror( errno ) ); return 0; default: { + if ( mDataCb ) + mDataCb( mBuf + mBuflen, ret ); + int old_scr = mTerm.scr; int old_histi = mTerm.histi; mTerm.scr = 0; diff --git a/src/tests/unit_tests/regex.cpp b/src/tests/unit_tests/regex.cpp index 6058a66ef..fbdbb6002 100644 --- a/src/tests/unit_tests/regex.cpp +++ b/src/tests/unit_tests/regex.cpp @@ -82,6 +82,7 @@ UTEST( LuaPattern, basicTest ) { EXPECT_EQ( start, 14 ); EXPECT_EQ( end, 16 ); } + RegExCache::destroySingleton(); } UTEST( RegExEngines, basicTest ) { @@ -96,4 +97,5 @@ UTEST( RegExEngines, basicTest ) { EXPECT_EQ( 38, matchesPCRE2[0].end ); EXPECT_EQ( 38, matchesOniguruma[0].start ); EXPECT_EQ( 38, matchesOniguruma[0].end ); + RegExCache::destroySingleton(); } diff --git a/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.cpp b/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.cpp index 1d389ea1b..9dc5e7d08 100644 --- a/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.cpp +++ b/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.cpp @@ -317,6 +317,8 @@ TerminalOutputRequest::TerminalOutputRequest( const json& body ) { sessionId = body.value( "sessionId", "" ); if ( body.contains( "terminalId" ) ) terminalId = body.value( "terminalId", "" ); + if ( body.contains( "outputByteLimit" ) && !body["outputByteLimit"].is_null() ) + outputByteLimit = body["outputByteLimit"].get(); } json TerminalOutputResponse::toJson() const { diff --git a/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.hpp b/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.hpp index 53668ab79..ab3209415 100644 --- a/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.hpp +++ b/src/tools/ecode/plugins/aiassistant/acp/acpprotocol.hpp @@ -249,6 +249,7 @@ struct TerminalExitStatus { struct TerminalOutputRequest { std::string sessionId; std::string terminalId; + std::optional outputByteLimit; TerminalOutputRequest() = default; TerminalOutputRequest( const json& body ); diff --git a/src/tools/ecode/plugins/aiassistant/acp/agentsession.cpp b/src/tools/ecode/plugins/aiassistant/acp/agentsession.cpp index 2ef064123..e6125dfee 100644 --- a/src/tools/ecode/plugins/aiassistant/acp/agentsession.cpp +++ b/src/tools/ecode/plugins/aiassistant/acp/agentsession.cpp @@ -24,6 +24,8 @@ bool AgentSession::start( const std::function& onReady ) { mClient->initialize( req, [this, onReady]( const InitializeResponse&, const std::optional& err ) { if ( err ) { + if ( onError ) + onError( *err ); if ( onReady ) onReady( false ); return; @@ -33,6 +35,8 @@ bool AgentSession::start( const std::function& onReady ) { mClient->newSession( nreq, [this, onReady]( const NewSessionResponse& nres, const std::optional& err ) { if ( err ) { + if ( onError ) + onError( *err ); if ( onReady ) onReady( false ); return; @@ -62,6 +66,8 @@ bool AgentSession::startLoaded( const std::string& sessionId, onReady]( const InitializeResponse& ires, const std::optional& err ) { if ( err ) { + if ( onError ) + onError( *err ); if ( onReady ) onReady( false ); return; @@ -74,6 +80,8 @@ bool AgentSession::startLoaded( const std::string& sessionId, lreq, [this, sessionId, onReady]( const LoadSessionResponse& lres, const std::optional& err ) { if ( err ) { + if ( onError ) + onError( *err ); if ( onReady ) onReady( false ); return; @@ -84,6 +92,8 @@ bool AgentSession::startLoaded( const std::string& sessionId, onReady( true ); } ); } else { + if ( onError ) + onError( { -1, "Agent does not support loading sessions" } ); if ( onReady ) onReady( false ); } @@ -106,7 +116,9 @@ void AgentSession::listSessions( ListSessionsRequest req; req.cwd = mClient->getConfig().workingDirectory; mClient->listSessions( - req, [cb]( const ListSessionsResponse& res, const std::optional& err ) { + req, [this, cb]( const ListSessionsResponse& res, const std::optional& err ) { + if ( err && onError ) + onError( *err ); if ( cb ) cb( res.sessions, err ); } ); @@ -124,11 +136,26 @@ void AgentSession::prompt( mClient->prompt( req, [this, cb]( const PromptResponse& res, const std::optional& err ) { mIsPrompting = false; + if ( err && onError ) + onError( *err ); if ( cb ) cb( res, err ); } ); } +void AgentSession::setConfigOption( + const SetConfigOptionRequest& req, + const std::function& )>& cb ) { + mClient->setConfigOption( req, [this, cb]( const SetConfigOptionResponse& res, + const std::optional& err ) { + if ( err && onError ) + onError( *err ); + if ( cb ) + cb( res, err ); + } ); +} + void AgentSession::cancel() { if ( mClient && !mSessionId.empty() ) { mClient->cancel( mSessionId ); @@ -136,8 +163,16 @@ void AgentSession::cancel() { } void AgentSession::setTerminalData( const std::string& terminalId, UITerminal* uiTerm ) { - mTerminals[terminalId] = - TermData{ uiTerm->getTerm(), uiTerm->getTerm()->getTerminal(), uiTerm }; + auto& termData = mTerminals[terminalId]; + termData = { uiTerm->getTerm(), uiTerm->getTerm()->getTerminal(), uiTerm, "" }; + if ( termData.emulator ) { + termData.emulator->setDataCb( [this, terminalId]( const char* data, size_t size ) { + auto it = mTerminals.find( terminalId ); + if ( it != mTerminals.end() ) { + it->second.outputBuffer.append( data, size ); + } + } ); + } } void AgentSession::setupClient() { @@ -196,8 +231,19 @@ void AgentSession::setupClient() { res.output = ""; res.truncated = false; auto it = mTerminals.find( req.terminalId ); - if ( it != mTerminals.end() && it->second.emulator ) { - if ( it->second.emulator->hasExited() ) { + if ( it != mTerminals.end() ) { + size_t limit = req.outputByteLimit.value_or( 0 ); + if ( limit > 0 && it->second.outputBuffer.size() > limit ) { + res.output = it->second.outputBuffer.substr( 0, limit ); + it->second.outputBuffer.erase( 0, limit ); + res.truncated = true; + } else { + res.output = std::move( it->second.outputBuffer ); + it->second.outputBuffer.clear(); + res.truncated = false; + } + + if ( it->second.emulator && it->second.emulator->hasExited() ) { TerminalExitStatus status; status.exitCode = it->second.emulator->getExitCode(); res.exitStatus = status; diff --git a/src/tools/ecode/plugins/aiassistant/acp/agentsession.hpp b/src/tools/ecode/plugins/aiassistant/acp/agentsession.hpp index 3cbfa8eeb..01140ee2f 100644 --- a/src/tools/ecode/plugins/aiassistant/acp/agentsession.hpp +++ b/src/tools/ecode/plugins/aiassistant/acp/agentsession.hpp @@ -29,6 +29,10 @@ class AgentSession { void prompt( const PromptRequest& req, const std::function& )>& cb ); + void setConfigOption( + const SetConfigOptionRequest& req, + const std::function& )>& cb ); void cancel(); bool isPrompting() const { return mIsPrompting; } @@ -59,6 +63,7 @@ class AgentSession { std::shared_ptr display; std::shared_ptr emulator; UITerminal* uiTerm{ nullptr }; + std::string outputBuffer; }; std::unordered_map mTerminals; diff --git a/src/tools/ecode/plugins/aiassistant/chatui.cpp b/src/tools/ecode/plugins/aiassistant/chatui.cpp index 5b82893d0..498c61d72 100644 --- a/src/tools/ecode/plugins/aiassistant/chatui.cpp +++ b/src/tools/ecode/plugins/aiassistant/chatui.cpp @@ -724,14 +724,12 @@ LLMChatUI::LLMChatUI( PluginManager* manager ) : return; auto inputUserRole = LLMChat::stringToRole( mChatUserRole ); - if ( ( !chats.empty() && - ( mChatInput->getDocument().isEmpty() || inputUserRole != LLMChat::Role::User ) ) || - ( chats.empty() && inputUserRole != LLMChat::Role::User ) ) { - if ( chats[chats.size() - 1] - ->findByClass( "role_ui" ) - ->asType() - ->getListBox() - ->getItemSelectedIndex() != 0 ) { + if ( !mIsAgentMode && ( ( !chats.empty() && ( mChatInput->getDocument().isEmpty() || + inputUserRole != LLMChat::Role::User ) ) || + ( chats.empty() && inputUserRole != LLMChat::Role::User ) ) ) { + auto rolePicker = chats[chats.size() - 1]->findByClass( "role_ui" ); + if ( rolePicker && + rolePicker->asType()->getListBox()->getItemSelectedIndex() != 0 ) { showMsg( getUISceneNode()->i18n( "llm_last_message_must_be_from_user", "The last chat message must be from a \"User\" role" ) ); @@ -1302,9 +1300,6 @@ void LLMChatUI::showChatHistory() { return; if ( err ) { loader->setVisible( false ); - NotificationCenter::instance()->addNotification( - uiSceneNode->i18n( "ai_assistant_agent_error", "Agent Error: " ) + - err->message ); win->asType()->closeWindow(); return; } @@ -1631,17 +1626,10 @@ void LLMChatUI::showAgentConfigWindow() { req.sessionId = mAgentSession->getSessionId(); req.configId = optId; req.optionId = subId; - mAgentSession->getClient()->setConfigOption( + mAgentSession->setConfigOption( req, [this, optId, subId]( const acp::SetConfigOptionResponse& res, const std::optional& err ) { - if ( err ) { - runOnMainThread( [this, err]() { - NotificationCenter::instance()->addNotification( - i18n( "agent_config_error", - "Failed to update agent config: " ) + - err->message ); - } ); - } else { + if ( !err ) { auto newOpts = res.configOptions; if ( newOpts.empty() ) { newOpts = acp::parseLegacyConfigOptions( @@ -1871,11 +1859,13 @@ void LLMChatUI::setupAgentSession() { addPlanBubble( planMarkdown ); } else if ( sessionUpdate == "available_commands_update" ) { if ( msg.contains( "availableCommands" ) && msg["availableCommands"].is_array() ) { - mAvailableCommands.clear(); - for ( const auto& cmd : msg["availableCommands"] ) { - mAvailableCommands.push_back( - { cmd.value( "name", "" ), cmd.value( "description", "" ) } ); - } + runOnMainThread( [this, msg] { + mAvailableCommands.clear(); + for ( const auto& cmd : msg["availableCommands"] ) { + mAvailableCommands.push_back( + { cmd.value( "name", "" ), cmd.value( "description", "" ) } ); + } + } ); } } };