-
Notifications
You must be signed in to change notification settings - Fork 1
feat(terminal): add user cancellation tracking and ANSI stripping for terminal output #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
039c09b
23f2bc4
3adf74c
9e13797
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,7 +37,7 @@ class ToolOrchestrator( | |||||||
| private val mcpConfigService: McpToolConfigService? = null, | ||||||||
| private val asyncShellExecution: Boolean = true | ||||||||
| ) { | ||||||||
| private val logger = getLogger("ToolOrchestrator") | ||||||||
| private val logger = getLogger("cc.unitmesh.agent.orchestrator.ToolOrchestrator") | ||||||||
|
|
||||||||
| // Coroutine scope for background tasks (async shell monitoring) | ||||||||
| private val backgroundScope = CoroutineScope(Dispatchers.Default) | ||||||||
|
|
@@ -113,7 +113,21 @@ class ToolOrchestrator( | |||||||
| // 启动 PTY 会话 | ||||||||
| liveSession = shellExecutor.startLiveExecution(command, shellConfig) | ||||||||
| logger.debug { "Live session started: ${liveSession.sessionId}" } | ||||||||
|
|
||||||||
|
|
||||||||
| // Register session to ShellSessionManager for cancel event handling | ||||||||
| val managedSession = cc.unitmesh.agent.tool.shell.ShellSessionManager.registerSession( | ||||||||
| sessionId = liveSession.sessionId, | ||||||||
| command = liveSession.command, | ||||||||
| workingDirectory = liveSession.workingDirectory, | ||||||||
| processHandle = liveSession.ptyHandle | ||||||||
| ) | ||||||||
| // Set process handlers from LiveShellSession | ||||||||
| managedSession.setProcessHandlers( | ||||||||
| isAlive = { liveSession.isAlive() }, | ||||||||
| kill = { liveSession.kill() } | ||||||||
| ) | ||||||||
| logger.debug { "Session registered to ShellSessionManager: ${liveSession.sessionId}" } | ||||||||
|
|
||||||||
| // 立即通知 renderer 添加 LiveTerminal(在执行之前!) | ||||||||
| logger.debug { "Adding LiveTerminal to renderer" } | ||||||||
| renderer.addLiveTerminal( | ||||||||
|
|
@@ -279,26 +293,65 @@ class ToolOrchestrator( | |||||||
| // Get output from ShellSessionManager (synced by UI's ProcessOutputCollector) | ||||||||
| // or fall back to LiveShellSession's stdout buffer | ||||||||
| val managedSession = cc.unitmesh.agent.tool.shell.ShellSessionManager.getSession(session.sessionId) | ||||||||
| val output = managedSession?.getOutput()?.ifEmpty { null } ?: session.getStdout() | ||||||||
| val rawOutput = managedSession?.getOutput()?.ifEmpty { null } ?: session.getStdout() | ||||||||
|
|
||||||||
| // Strip ANSI escape sequences for clean output to AI | ||||||||
| val output = cc.unitmesh.agent.tool.shell.AnsiStripper.stripAndNormalize(rawOutput) | ||||||||
|
|
||||||||
| // Update renderer with final status | ||||||||
| // Check if this was a user cancellation | ||||||||
| val wasCancelledByUser = managedSession?.cancelledByUser == true | ||||||||
|
|
||||||||
| // Update renderer with final status (including cancellation info) | ||||||||
| renderer.updateLiveTerminalStatus( | ||||||||
| sessionId = session.sessionId, | ||||||||
| exitCode = exitCode, | ||||||||
| executionTimeMs = executionTimeMs, | ||||||||
| output = output | ||||||||
| output = output, | ||||||||
| cancelledByUser = wasCancelledByUser | ||||||||
| ) | ||||||||
|
|
||||||||
| logger.debug { "Updated renderer with session completion: ${session.sessionId}" } | ||||||||
| } catch (e: Exception) { | ||||||||
| logger.error(e) { "Error monitoring session ${session.sessionId}: ${e.message}" } | ||||||||
|
|
||||||||
| // Check if this was a user cancellation and get output from managedSession | ||||||||
| val managedSession = cc.unitmesh.agent.tool.shell.ShellSessionManager.getSession(session.sessionId) | ||||||||
| val wasCancelledByUser = managedSession?.cancelledByUser == true | ||||||||
|
|
||||||||
| logger.debug { "managedSession for ${session.sessionId}: ${managedSession != null}, cancelledByUser: $wasCancelledByUser" } | ||||||||
|
|
||||||||
| // Get output from managedSession (which was synced during waitForSession) | ||||||||
| // or fall back to LiveShellSession's stdout buffer | ||||||||
| val managedOutput = managedSession?.getOutput() | ||||||||
| val sessionOutput = session.getStdout() | ||||||||
| val rawOutput = managedOutput?.ifEmpty { null } ?: sessionOutput | ||||||||
|
|
||||||||
| // Strip ANSI escape sequences for clean output to AI | ||||||||
| val capturedOutput = rawOutput?.let { | ||||||||
| cc.unitmesh.agent.tool.shell.AnsiStripper.stripAndNormalize(it) | ||||||||
| } | ||||||||
|
|
||||||||
| logger.debug { "Output sources - managedOutput length: ${managedOutput?.length ?: 0}, sessionOutput length: ${sessionOutput.length}, capturedOutput length: ${capturedOutput?.length ?: 0}" } | ||||||||
|
|
||||||||
| // Build error message with captured output | ||||||||
| val errorOutput = buildString { | ||||||||
| appendLine("Error: ${e.message}") | ||||||||
| if (!capturedOutput.isNullOrEmpty()) { | ||||||||
| appendLine() | ||||||||
| appendLine("Output before error:") | ||||||||
| appendLine(capturedOutput) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| logger.debug { "Final errorOutput length: ${errorOutput.length}" } | ||||||||
|
|
||||||||
| // Update renderer with error status | ||||||||
| renderer.updateLiveTerminalStatus( | ||||||||
| sessionId = session.sessionId, | ||||||||
| exitCode = -1, | ||||||||
| executionTimeMs = Clock.System.now().toEpochMilliseconds() - startTime, | ||||||||
| output = "Error: ${e.message}" | ||||||||
| output = errorOutput, | ||||||||
| cancelledByUser = wasCancelledByUser | ||||||||
| ) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
@@ -317,7 +370,7 @@ class ToolOrchestrator( | |||||||
|
|
||||||||
| // Use new ExecutableTool architecture for most tools | ||||||||
| // Only special-case tools that need custom handling (shell with PTY, etc.) | ||||||||
| return when (val toolType = toolName.toToolType()) { | ||||||||
| return when (toolName.toToolType()) { | ||||||||
|
||||||||
| return when (toolName.toToolType()) { | |
| val toolType = toolName.toToolType() | |
| return when (toolType) { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| package cc.unitmesh.agent.tool.shell | ||
|
|
||
| /** | ||
| * Utility object for stripping ANSI escape sequences from terminal output. | ||
| * This converts raw terminal output with color codes, cursor movements, etc. | ||
| * into clean, readable ASCII text. | ||
| */ | ||
| object AnsiStripper { | ||
| private const val ESC = '\u001B' | ||
|
|
||
| /** | ||
| * Strip all ANSI escape sequences from the given text. | ||
| * Handles: | ||
| * - CSI sequences (ESC[...X) - colors, cursor movement, erase | ||
| * - OSC sequences (ESC]...BEL/ST) - window title, etc. | ||
| * - Simple escape sequences (ESC X) | ||
| * | ||
| * @param text The text containing ANSI escape sequences | ||
| * @return Clean text with all escape sequences removed | ||
| */ | ||
| fun strip(text: String): String { | ||
| if (!text.contains(ESC)) { | ||
| return text | ||
| } | ||
|
|
||
| val result = StringBuilder() | ||
| var i = 0 | ||
|
|
||
| while (i < text.length) { | ||
| val ch = text[i] | ||
|
|
||
| when { | ||
| ch == ESC && i + 1 < text.length -> { | ||
| val next = text[i + 1] | ||
| when (next) { | ||
| '[' -> { | ||
| // CSI sequence: ESC[...X (ends with a letter) | ||
| i = skipCsiSequence(text, i + 2) | ||
| } | ||
| ']' -> { | ||
| // OSC sequence: ESC]...BEL or ESC]...ST | ||
| i = skipOscSequence(text, i + 2) | ||
| } | ||
| '(', ')' -> { | ||
| // Character set selection: ESC(X or ESC)X | ||
| i = if (i + 2 < text.length) i + 3 else text.length | ||
| } | ||
| else -> { | ||
| // Simple escape sequence: ESC X | ||
| i += 2 | ||
| } | ||
| } | ||
| } | ||
| ch == '\r' -> { | ||
| // Carriage return - skip it (will be handled with newlines) | ||
| i++ | ||
| } | ||
| else -> { | ||
| result.append(ch) | ||
| i++ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return result.toString() | ||
| } | ||
|
|
||
| /** | ||
| * Skip a CSI sequence starting at the given position. | ||
| * CSI sequences end with a letter (0x40-0x7E). | ||
| */ | ||
| private fun skipCsiSequence(text: String, start: Int): Int { | ||
| var i = start | ||
| while (i < text.length) { | ||
| val ch = text[i] | ||
| if (ch in '@'..'~') { | ||
| // Found the terminating character | ||
| return i + 1 | ||
| } | ||
| i++ | ||
| } | ||
| return text.length | ||
| } | ||
|
|
||
| /** | ||
| * Skip an OSC sequence starting at the given position. | ||
| * OSC sequences end with BEL (0x07) or ST (ESC\). | ||
| */ | ||
| private fun skipOscSequence(text: String, start: Int): Int { | ||
| var i = start | ||
| while (i < text.length) { | ||
| val ch = text[i] | ||
| when { | ||
| ch == '\u0007' -> { | ||
| // BEL character terminates OSC | ||
| return i + 1 | ||
| } | ||
| ch == ESC && i + 1 < text.length && text[i + 1] == '\\' -> { | ||
| // ST (String Terminator) terminates OSC | ||
| return i + 2 | ||
| } | ||
| } | ||
| i++ | ||
| } | ||
| return text.length | ||
| } | ||
|
|
||
| /** | ||
| * Strip ANSI sequences and also normalize line endings. | ||
| * Converts \r\n to \n and removes standalone \r. | ||
| */ | ||
| fun stripAndNormalize(text: String): String { | ||
| return strip(text) | ||
| .replace("\r\n", "\n") | ||
| .replace("\r", "") | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Excessive Debug Logging: Multiple debug log statements (lines 321, 334, 346) are added for troubleshooting but seem verbose for production code. These logs provide very detailed debugging information that should typically be temporary.
Recommendation: Consider removing or simplifying these debug logs once the feature is stable, or use trace level logging instead of debug for this level of detail.