diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index fb4316046d..db39981285 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -39,6 +39,7 @@ from openhands.sdk.event import ( ActionEvent, AgentErrorEvent, + Condensation, CondensationRequest, Event, EventID, @@ -454,7 +455,9 @@ def _default_callback(e): # This runs on first run()/send_message() call and handles both # explicit hooks and plugin hooks in one place self._hook_processor = None - self._on_event = self._tree_stamping(self._rules_injecting(base_callback)) + self._on_event = self._tree_stamping( + self._activation_state_syncing(self._rules_injecting(base_callback)) + ) self._on_token = ( BaseConversation.compose_callbacks(token_callbacks) if token_callbacks @@ -555,6 +558,51 @@ def wrapped(event: Event) -> None: return cast(ConversationCallbackType, wrapped) + def _activation_state_syncing( + self, inner: ConversationCallbackType + ) -> ConversationCallbackType: + """Wrap a callback to rebuild trigger state after condensation.""" + + def wrapped(event: Event) -> None: + inner(event) + if isinstance(event, Condensation): + self._rebuild_skill_activation_state() + + return cast(ConversationCallbackType, wrapped) + + def _rebuild_skill_activation_state(self) -> None: + """Rebuild trigger deduplication state from the current conversation view.""" + agent_context = self.agent.agent_context + activated_knowledge_skills: list[str] = [] + activated_path_rules: list[str] = [] + + for event in self._state.view.events: + if isinstance(event, MessageEvent): + activated_names = event.activated_skills + elif isinstance(event, ObservationEvent) and event.extended_content: + if agent_context is None: + continue + file_path = self._touched_rule_path(event) + if file_path is None: + continue + result = agent_context.get_tool_use_suffix( + file_path=file_path, + skip_skill_names=[], + ) + activated_names = result[1] if result is not None else [] + else: + continue + + for name in activated_names: + if isinstance(event, MessageEvent): + if name not in activated_knowledge_skills: + activated_knowledge_skills.append(name) + elif name not in activated_path_rules: + activated_path_rules.append(name) + + self._state.activated_knowledge_skills = activated_knowledge_skills + self._state.activated_path_rules = activated_path_rules + def _maybe_inject_path_rules(self, event: Event) -> Event: """Return ``event`` with matching path-rule content, or unchanged. @@ -1250,7 +1298,9 @@ def _ensure_plugins_loaded(self) -> None: visualizer=self._visualizer, conversation_stats=self._state.stats, ) - self._on_event = self._tree_stamping(self._rules_injecting(raw_on_event)) + self._on_event = self._tree_stamping( + self._activation_state_syncing(self._rules_injecting(raw_on_event)) + ) self._hook_processor.set_conversation_state(self._state) self._hook_processor.run_session_start() @@ -1323,7 +1373,9 @@ def _merge_runtime_plugin_hooks(self, plugin_hooks: HookConfig) -> None: visualizer=self._visualizer, conversation_stats=self._state.stats, ) - self._on_event = self._tree_stamping(self._rules_injecting(raw_on_event)) + self._on_event = self._tree_stamping( + self._activation_state_syncing(self._rules_injecting(raw_on_event)) + ) self._hook_processor.set_conversation_state(self._state) self._hook_processor.run_session_start() @@ -1847,9 +1899,6 @@ def send_message(self, message: str | Message, sender: str | None = None) -> Non # We skip skills that were already activated skip_skill_names=self._state.activated_knowledge_skills, ) - # TODO(calvin): we need to update - # self._state.activated_knowledge_skills - # so condenser can work if ctx: content, activated_skill_names = ctx logger.debug( diff --git a/tests/sdk/conversation/test_triggered_skills_after_condensation.py b/tests/sdk/conversation/test_triggered_skills_after_condensation.py new file mode 100644 index 0000000000..3bcf66aca4 --- /dev/null +++ b/tests/sdk/conversation/test_triggered_skills_after_condensation.py @@ -0,0 +1,170 @@ +"""Regression tests for trigger-based skills across conversation condensation.""" + +from pathlib import Path + +from openhands.sdk.agent import Agent +from openhands.sdk.context.agent_context import AgentContext +from openhands.sdk.context.condenser import LLMSummarizingCondenser +from openhands.sdk.conversation import LocalConversation +from openhands.sdk.event import ActionEvent, MessageEvent, ObservationEvent +from openhands.sdk.llm import Message, MessageToolCall, TextContent +from openhands.sdk.skills import KeywordTrigger, PathTrigger, Skill +from openhands.sdk.testing import TestLLM +from openhands.sdk.tool.builtins.finish import FinishObservation +from openhands.sdk.tool.schema import Action + + +class _CondensationFileAction(Action): + path: str + command: str = "view" + + +def _message(text: str) -> Message: + return Message(role="assistant", content=[TextContent(text=text)]) + + +def _conversation(tmp_path: Path, skills: list[Skill]) -> LocalConversation: + agent = Agent( + llm=TestLLM.from_messages([_message("agent response")]), + condenser=LLMSummarizingCondenser( + llm=TestLLM.from_messages([_message("condensed history")]), + max_size=20, + keep_first=2, + ), + tools=[], + include_default_tools=[], + agent_context=AgentContext(skills=skills), + ) + return LocalConversation( + agent=agent, + workspace=tmp_path, + persistence_dir=tmp_path / "conversation", + delete_on_close=True, + ) + + +def _append_path_observation( + conversation: LocalConversation, path: Path, tool_call_id: str +) -> ObservationEvent: + action = ActionEvent( + thought=[TextContent(text="inspect file")], + action=_CondensationFileAction(path=str(path)), + tool_name="file_editor", + tool_call_id=tool_call_id, + tool_call=MessageToolCall( + id=tool_call_id, + name="file_editor", + arguments="{}", + origin="completion", + ), + llm_response_id=f"response-{tool_call_id}", + source="agent", + ) + observation = ObservationEvent( + observation=FinishObservation(content=[TextContent(text="file contents")]), + action_id=action.id, + tool_name=action.tool_name, + tool_call_id=tool_call_id, + ) + with conversation._state: + conversation._on_event(action) + conversation._on_event(observation) + + persisted = [ + event + for event in conversation.state.events + if isinstance(event, ObservationEvent) and event.action_id == action.id + ] + assert persisted + return persisted[-1] + + +def test_keyword_skill_can_reactivate_after_its_event_is_condensed( + tmp_path: Path, +) -> None: + skill = Skill( + name="python_tips", + content="Prefer small, focused Python functions.", + trigger=KeywordTrigger(keywords=["python"]), + ) + conversation = _conversation(tmp_path, [skill]) + try: + for index in range(5): + conversation.send_message(f"prefix message {index}") + conversation.send_message("Show me a python example") + triggered_event = conversation.state.events[-1] + assert isinstance(triggered_event, MessageEvent) + assert triggered_event.activated_skills == ["python_tips"] + + conversation.send_message("Show me another python example") + deduped_event = conversation.state.events[-1] + assert isinstance(deduped_event, MessageEvent) + assert deduped_event.activated_skills == [] + + for index in range(10): + conversation.send_message(f"tail message {index}") + + conversation.condense() + + assert triggered_event.id not in { + event.id for event in conversation.state.view.events + } + assert conversation.state.activated_knowledge_skills == [] + + conversation.send_message("Show me another python example") + retriggered_event = conversation.state.events[-1] + assert isinstance(retriggered_event, MessageEvent) + assert retriggered_event.activated_skills == ["python_tips"] + assert any( + "Prefer small, focused Python functions." in content.text + for content in retriggered_event.extended_content + ) + finally: + conversation.close() + + +def test_path_rule_can_reactivate_after_its_event_is_condensed( + tmp_path: Path, +) -> None: + rule = Skill( + name="typescript_rules", + content="Keep TypeScript modules narrowly scoped.", + trigger=PathTrigger(paths=["src/**/*.ts"]), + ) + conversation = _conversation(tmp_path, [rule]) + try: + for index in range(6): + conversation.send_message(f"prefix message {index}") + first_observation = _append_path_observation( + conversation, tmp_path / "src" / "app.ts", "first-call" + ) + assert any( + "Keep TypeScript modules narrowly scoped." in content.text + for content in first_observation.extended_content + ) + + deduped_observation = _append_path_observation( + conversation, tmp_path / "src" / "second.ts", "deduped-call" + ) + assert deduped_observation.extended_content == [] + + for index in range(10): + conversation.send_message(f"tail message {index}") + + conversation.condense() + + assert first_observation.id not in { + event.id for event in conversation.state.view.events + } + assert conversation.state.activated_path_rules == [] + + second_observation = _append_path_observation( + conversation, tmp_path / "src" / "another.ts", "second-call" + ) + assert any( + "Keep TypeScript modules narrowly scoped." in content.text + for content in second_observation.extended_content + ) + assert conversation.state.activated_path_rules == ["typescript_rules"] + finally: + conversation.close()