Repository: All-Hands-AI/OpenHands
Pull request: [agent] improve finish tool for sonnet 3.7
- [ ] This change is worth documenting at https://docs.all-hands.dev/
- [ ] Include this change in the Release Notes. If checked, you **must** provide an **end-user friendly** description for your change below

**End-user friendly description of the problem this fixes or functionality that this introduces.**

Improve agent's ability to declare task completion

---
**Give a summary of what the PR does, explaining any non-trivial design decisions.**

New sonnet really like to ends the convo by sending "FinishAction" without actual content. 
https://openhands-ai.slack.com/archives/C07S376KASK/p1740595180980119



I suspect it was expecting an actual argument to put in the message in FinishAction -- so i made this PR. A bonus now is that i also ask the agent for "predict task is completed or not" - in case this is useful in the future

How that looks in UI:

<img width="599" alt="image" src="https://github.com/user-attachments/assets/d45e3cc1-aa12-4cc8-9a6a-efbf3d7385f7" />

---
**Link of any specific issues this addresses.**

Fix https://github.com/All-Hands-AI/OpenHands/issues/6406
---

To run this PR locally, use the following command:
```
docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:cd7bcc3-nikolaik   --name openhands-app-cd7bcc3   docker.all-hands.dev/all-hands-ai/openhands:cd7bcc3
```

Problem statement:
[Bug]: The LLM doesn't understand the Finish tool very well
### Is there an existing issue for the same bug?

- [x] I have checked the existing issues.

### Describe the bug and reproduction steps

Looking at trajectories, specially on evals, but also on those in the resolver I think, I seem to see too often that when the LLM is happy with the job, it sends a MessageAction, not a FinishAction.

We insert then a fake user message to remind it to can call the Finish tool if it's done, and then it calls the Finish tool. That means it uses one more iteration than expected, often.

Maybe we can explain better or more clear that it's supposed to call Finish when it thinks the task is done.

### OpenHands Installation

Docker command in README

### OpenHands Version

_No response_

### Operating System

None

### Logs, Errors, Screenshots, and Additional Context

_No response_

Hints from issue discussion:
This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Commit message: fix logging error: --- Logging error ---
Traceback (most recent call last):
  File "/home/xingyaow/OpenHands-dev/evaluation/utils/shared.py", line 323, in _process_instance_wrapper
    result = process_instance_func(instance, metadata, use_mp, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/xingyaow/OpenHands-dev/evaluation/benchmarks/swe_bench/run_infer.py", line 461, in process_instance
    raise EvalException('Fatal error detected: ' + state.last_error)
evaluation.utils.shared.EvalException: Fatal error detected: ConnectionError: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/logging/__init__.py", line 1160, in emit
    msg = self.format(record)
          ^^^^^^^^^^^^^^^^^^^
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/logging/__init__.py", line 999, in format
    return fmt.format(record)
           ^^^^^^^^^^^^^^^^^^
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/logging/__init__.py", line 719, in format
    s = s + self.formatStack(record.stack_info)
        ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
TypeError: can only concatenate str (not "bool") to str
Call stack:
  File "/home/xingyaow/OpenHands-dev/evaluation/benchmarks/swe_bench/run_infer.py", line 584, in <module>
    run_evaluation(
  File "/home/xingyaow/OpenHands-dev/evaluation/utils/shared.py", line 415, in run_evaluation
    with mp.Pool(num_workers) as pool:
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/context.py", line 119, in Pool
    return Pool(processes, initializer, initargs, maxtasksperchild,
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/pool.py", line 215, in __init__
    self._repopulate_pool()
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/pool.py", line 306, in _repopulate_pool
    return self._repopulate_pool_static(self._ctx, self.Process,
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/pool.py", line 329, in _repopulate_pool_static
    w.start()
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/process.py", line 121, in start
    self._popen = self._Popen(self)
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/context.py", line 282, in _Popen
    return Popen(process_obj)
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/popen_fork.py", line 19, in __init__
    self._launch(process_obj)
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/popen_fork.py", line 71, in _launch
    code = process_obj._bootstrap(parent_sentinel=child_r)
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/home/xingyaow/micromamba/envs/oh/lib/python3.12/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/home/xingyaow/OpenHands-dev/evaluation/utils/shared.py", line 384, in _process_instance_wrapper_mp
    return _process_instance_wrapper(*args)
  File "/home/xingyaow/OpenHands-dev/evaluation/utils/shared.py", line 378, in _process_instance_wrapper
    logger.error(msg)
Message: '----------\nError in instance [pydata__xarray-7233]: Fatal error detected: ConnectionError: (\'Connection aborted.\', RemoteDisconnected(\'Remote end closed connection without response\')). Stacktrace:\nTraceback (most recent call last):\n  File "/home/xingyaow/OpenHands-dev/evaluation/utils/shared.py", line 323, in _process_instance_wrapper\n    result = process_instance_func(instance, metadata, use_mp, **kwargs)\n             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File "/home/xingyaow/OpenHands-dev/evaluation/benchmarks/swe_bench/run_infer.py", line 461, in process_instance\n    raise EvalException(\'Fatal error detected: \' + state.last_error)\nevaluation.utils.shared.EvalException: Fatal error detected: ConnectionError: (\'Connection aborted.\', RemoteDisconnected(\'Remote end closed connection without response\'))\n\n----------[The above error occurred. Retrying... (attempt 1 of 5)]----------\n'
Arguments: ()
Instance pydata__xarray-7233 - 2025-02-27 23:04:06,218 - WARNING - Using o

Patch to review:
diff --git a/frontend/src/services/actions.ts b/frontend/src/services/actions.ts
index d6702aad088f..bce06c40218e 100644
--- a/frontend/src/services/actions.ts
+++ b/frontend/src/services/actions.ts
@@ -57,6 +57,28 @@ const messageActions = {
       store.dispatch(appendJupyterInput(message.args.code));
     }
   },
+  [ActionType.FINISH]: (message: ActionMessage) => {
+    store.dispatch(addAssistantMessage(message.args.message_text));
+    let successPrediction = "";
+    if (message.args.task_completed === "partial") {
+      successPrediction =
+        "The agent thinks that the task was **completed partially**.";
+    } else if (message.args.task_completed === "false") {
+      successPrediction =
+        "The agent thinks that the task was **not completed**.";
+    } else if (message.args.task_completed === "true") {
+      successPrediction =
+        "The agent thinks that the task was **completed successfully**.";
+    }
+    if (successPrediction) {
+      // if message_text is not empty, add a new line before the success prediction
+      if (message.args.message_text) {
+        store.dispatch(addAssistantMessage(`\n${successPrediction}`));
+      } else {
+        store.dispatch(addAssistantMessage(successPrediction));
+      }
+    }
+  },
 };
 
 export function handleActionMessage(message: ActionMessage) {
diff --git a/frontend/src/types/core/actions.ts b/frontend/src/types/core/actions.ts
index e7836f885784..393136647f8e 100644
--- a/frontend/src/types/core/actions.ts
+++ b/frontend/src/types/core/actions.ts
@@ -51,6 +51,8 @@ export interface ThinkAction extends OpenHandsActionEvent<"think"> {
 export interface FinishAction extends OpenHandsActionEvent<"finish"> {
   source: "agent";
   args: {
+    message_text: string;
+    task_completed: "success" | "failure" | "partial";
     outputs: Record<string, unknown>;
     thought: string;
   };
diff --git a/openhands/agenthub/codeact_agent/function_calling.py b/openhands/agenthub/codeact_agent/function_calling.py
index fe5f08eb8301..7052a8b57fc3 100644
--- a/openhands/agenthub/codeact_agent/function_calling.py
+++ b/openhands/agenthub/codeact_agent/function_calling.py
@@ -108,7 +108,10 @@ def response_to_actions(response: ModelResponse) -> list[Action]:
             # AgentFinishAction
             # ================================================
             elif tool_call.function.name == FinishTool['function']['name']:
-                action = AgentFinishAction()
+                action = AgentFinishAction(
+                    message_text=arguments.get('message', ''),
+                    task_completed=arguments.get('task_completed', None),
+                )
 
             # ================================================
             # LLMBasedFileEditTool (LLM-based file editor, deprecated)
diff --git a/openhands/agenthub/codeact_agent/tools/finish.py b/openhands/agenthub/codeact_agent/tools/finish.py
index 2cb081ace50b..dd3292f3edc9 100644
--- a/openhands/agenthub/codeact_agent/tools/finish.py
+++ b/openhands/agenthub/codeact_agent/tools/finish.py
@@ -1,11 +1,39 @@
 from litellm import ChatCompletionToolParam, ChatCompletionToolParamFunctionChunk
 
-_FINISH_DESCRIPTION = """Finish the interaction when the task is complete OR if the assistant cannot proceed further with the task."""
+_FINISH_DESCRIPTION = """Signals the completion of the current task or conversation.
+
+Use this tool when:
+- You have successfully completed the user's requested task
+- You cannot proceed further due to technical limitations or missing information
+
+The message should include:
+- A clear summary of actions taken and their results
+- Any next steps for the user
+- Explanation if you're unable to complete the task
+- Any follow-up questions if more information is needed
+
+The task_completed field should be set to True if you believed you have completed the task, and False otherwise.
+"""
 
 FinishTool = ChatCompletionToolParam(
     type='function',
     function=ChatCompletionToolParamFunctionChunk(
         name='finish',
         description=_FINISH_DESCRIPTION,
+        parameters={
+            'type': 'object',
+            'required': ['message', 'task_completed'],
+            'properties': {
+                'message': {
+                    'type': 'string',
+                    'description': 'Final message to send to the user',
+                },
+                'task_completed': {
+                    'type': 'string',
+                    'enum': ['true', 'false', 'partial'],
+                    'description': 'Whether you have completed the task.',
+                },
+            },
+        },
     ),
 )
diff --git a/openhands/core/logger.py b/openhands/core/logger.py
index 9820ab564d4f..16c59645b65b 100644
--- a/openhands/core/logger.py
+++ b/openhands/core/logger.py
@@ -77,7 +77,8 @@ class StackInfoFilter(logging.Filter):
     def filter(self, record: logging.LogRecord) -> bool:
         if record.levelno >= logging.ERROR:
             # LogRecord attributes are dynamically typed
-            setattr(record, 'stack_info', True)
+
+            setattr(record, 'stack_info', ''.join(traceback.format_stack()))
             setattr(record, 'exc_info', sys.exc_info())
         return True
 
diff --git a/openhands/events/action/agent.py b/openhands/events/action/agent.py
index e408007de3e9..e8831e39e3c5 100644
--- a/openhands/events/action/agent.py
+++ b/openhands/events/action/agent.py
@@ -1,4 +1,5 @@
 from dataclasses import dataclass, field
+from enum import Enum
 from typing import Any
 
 from openhands.core.schema import ActionType
@@ -33,16 +34,26 @@ def __str__(self) -> str:
         return ret
 
 
+class AgentFinishTaskCompleted(Enum):
+    FALSE = 'false'
+    PARTIAL = 'partial'
+    TRUE = 'true'
+
+
 @dataclass
 class AgentFinishAction(Action):
     """An action where the agent finishes the task.
 
     Attributes:
-        outputs (dict): The outputs of the agent, for instance "content".
+        message_text (str): The message to send to the user.
+        task_completed (enum): Whether the agent believes the task has been completed.
+        outputs (dict): The other outputs of the agent, for instance "content".
         thought (str): The agent's explanation of its actions.
         action (str): The action type, namely ActionType.FINISH.
     """
 
+    message_text: str = ''
+    task_completed: AgentFinishTaskCompleted | None = None
     outputs: dict[str, Any] = field(default_factory=dict)
     thought: str = ''
     action: str = ActionType.FINISH
diff --git a/tests/unit/test_action_serialization.py b/tests/unit/test_action_serialization.py
index 84eb03148454..6dbb3c3f2962 100644
--- a/tests/unit/test_action_serialization.py
+++ b/tests/unit/test_action_serialization.py
@@ -84,12 +84,23 @@ def test_message_action_serialization_deserialization():
 
 
 def test_agent_finish_action_serialization_deserialization():
-    original_action_dict = {'action': 'finish', 'args': {'outputs': {}, 'thought': ''}}
+    original_action_dict = {
+        'action': 'finish',
+        'args': {
+            'outputs': {},
+            'thought': '',
+            'task_completed': None,
+            'message_text': '',
+        },
+    }
     serialization_deserialization(original_action_dict, AgentFinishAction)
 
 
 def test_agent_reject_action_serialization_deserialization():
-    original_action_dict = {'action': 'reject', 'args': {'outputs': {}, 'thought': ''}}
+    original_action_dict = {
+        'action': 'reject',
+        'args': {'outputs': {}, 'thought': ''},
+    }
     serialization_deserialization(original_action_dict, AgentRejectAction)
 
 
diff --git a/tests/unit/test_security.py b/tests/unit/test_security.py
index b3d7ce748db1..61b172e989a2 100644
--- a/tests/unit/test_security.py
+++ b/tests/unit/test_security.py
@@ -347,7 +347,11 @@ async def test_unsafe_bash_command(temp_dir: str):
                     type='function',
                     function=Function(
                         name=ActionType.FINISH,
-                        arguments={'outputs': {'content': 'outputs content'}},
+                        arguments={
+                            'outputs': {'content': 'outputs content'},
+                            'task_completed': None,
+                            'message_text': '',
+                        },
                     ),
                 ),
             ],


Code review comments:
diff_hunk: @@ -108,7 +108,10 @@ def response_to_actions(response: ModelResponse) -> list[Action]:
             # AgentFinishAction
             # ================================================
             elif tool_call.function.name == FinishTool['function']['name']:
-                action = AgentFinishAction()
+                action = AgentFinishAction(
+                    message_text=arguments.get('message', ''),
path: openhands/agenthub/codeact_agent/function_calling.py
text: @user1:
Aha this `message_text` to `message` must have been due to that property?

🤔
What if we let `message` be sent to the LLM (it's already evaluated), but name `message_text`... differently for us and for saving in the stream? Just thinking it's already `message_text` vs `message`... though I don't know what it can be, `final_thought` or `model_message`... e.g. message from the model (I have a lot of bad ideas)

@author:
yeah i'm not super satisfied with the name "message_text" either 😓  open to other good ideas. I'll probably sleep on it and decide tmr!

@author:
yeah i like `final_thought`! will change it to that now