ACP Support Improvement for ecode (Terminal output, better error reporting). ACP plans.

Safety memory clean ups.

.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<void(const char*, size_t)>;`
+    - 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.

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<Row> 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<Row> 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<unsigned int, std::unique_ptr<Page>>
 		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

include/eepp/system/resourcemanager.hpp

@@ -316,7 +316,15 @@ template <class T> T* ResourceManagerMulti<T>::add( T* resource ) {
 
 template <class T> bool ResourceManagerMulti<T>::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 );

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 );

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<Uint8>().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<FT_Face>( mFace );
+
+	if ( !face || !face->size )
+		return false;
+
 	FT_UShort currentSize = face->size->metrics.x_ppem;
 
 	if ( currentSize != characterSize ) {

src/modules/eterm/include/eterm/terminal/terminalemulator.hpp

@@ -254,6 +254,10 @@ class TerminalEmulator final {
 		mPromptStateChangedCb = promptStateChangedCb;
 	}
 
+	using DataCb = std::function<void( const char*, size_t )>;
+
+	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 );
 

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;

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();
 }

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<uint64_t>();
 }
 
 json TerminalOutputResponse::toJson() const {

src/tools/ecode/plugins/aiassistant/acp/acpprotocol.hpp

@@ -249,6 +249,7 @@ struct TerminalExitStatus {
 struct TerminalOutputRequest {
 	std::string sessionId;
 	std::string terminalId;
+	std::optional<uint64_t> outputByteLimit;
 
 	TerminalOutputRequest() = default;
 	TerminalOutputRequest( const json& body );

src/tools/ecode/plugins/aiassistant/acp/agentsession.cpp

@@ -24,6 +24,8 @@ bool AgentSession::start( const std::function<void( bool )>& onReady ) {
 		mClient->initialize( req, [this, onReady]( const InitializeResponse&,
 												   const std::optional<ResponseError>& err ) {
 			if ( err ) {
+				if ( onError )
+					onError( *err );
 				if ( onReady )
 					onReady( false );
 				return;
@@ -33,6 +35,8 @@ bool AgentSession::start( const std::function<void( bool )>& onReady ) {
 			mClient->newSession( nreq, [this, onReady]( const NewSessionResponse& nres,
 														const std::optional<ResponseError>& 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<ResponseError>& 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<ResponseError>& 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<ResponseError>& err ) {
+		req, [this, cb]( const ListSessionsResponse& res, const std::optional<ResponseError>& 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<ResponseError>& err ) {
 			mIsPrompting = false;
+			if ( err && onError )
+				onError( *err );
 			if ( cb )
 				cb( res, err );
 		} );
 }
 
+void AgentSession::setConfigOption(
+	const SetConfigOptionRequest& req,
+	const std::function<void( const SetConfigOptionResponse&,
+							  const std::optional<ResponseError>& )>& cb ) {
+	mClient->setConfigOption( req, [this, cb]( const SetConfigOptionResponse& res,
+											   const std::optional<ResponseError>& 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;

src/tools/ecode/plugins/aiassistant/acp/agentsession.hpp

@@ -29,6 +29,10 @@ class AgentSession {
 	void prompt( const PromptRequest& req,
 				 const std::function<void( const PromptResponse&,
 										   const std::optional<ResponseError>& )>& cb );
+	void setConfigOption(
+		const SetConfigOptionRequest& req,
+		const std::function<void( const SetConfigOptionResponse&,
+								  const std::optional<ResponseError>& )>& cb );
 	void cancel();
 
 	bool isPrompting() const { return mIsPrompting; }
@@ -59,6 +63,7 @@ class AgentSession {
 		std::shared_ptr<TerminalDisplay> display;
 		std::shared_ptr<TerminalEmulator> emulator;
 		UITerminal* uiTerm{ nullptr };
+		std::string outputBuffer;
 	};
 	std::unordered_map<std::string, TermData> mTerminals;
 

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<UIDropDownList>()
-					 ->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<UIDropDownList>()->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<UIWindow>()->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<acp::ResponseError>& 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", "" ) } );
+					}
+				} );
 			}
 		}
 	};