diff --git a/backend/packages/harness/deerflow/agents/middlewares/skill_context.py b/backend/packages/harness/deerflow/agents/middlewares/skill_context.py index 4923f13106..e7943ea7fa 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/skill_context.py +++ b/backend/packages/harness/deerflow/agents/middlewares/skill_context.py @@ -140,25 +140,36 @@ def extract_skills( skills_root: str, read_tool_names: Collection[str], ) -> list[SkillEntry]: - """Enumerate skill-file reads (AI read_file call + paired ToolMessage result).""" + """Enumerate skill-file reads (AI read_file call + paired ToolMessage result). + + A result pairs only with a read call from the nearest preceding AIMessage, + because tool-call ids are unique per response, not per conversation; a + result that ended up non-adjacent to its call is dropped. + """ normalized_root = posixpath.normpath(skills_root.rstrip("/") or "/") read_names = frozenset(read_tool_names) - skill_paths_by_id: dict[str, str] = {} - for message in messages: - if not isinstance(message, AIMessage): - continue - for tool_call in message.tool_calls or []: - if _tool_call_name(tool_call) not in read_names: - continue - tool_call_id = _tool_call_id(tool_call) - raw_path = _tool_call_path(tool_call) - path = _normalize_under_root(raw_path, normalized_root) if raw_path else None - if tool_call_id and path and _is_skill_file(path): - skill_paths_by_id[tool_call_id] = path - entries: list[SkillEntry] = [] + # Tool-call ids are unique per assistant response, not per conversation, so + # this map is rebuilt from each AIMessage's own calls and a ToolMessage is + # resolved against the nearest preceding response — the pairing rule + # ``tool_call_args.pair_tool_call_results`` applies. A single map for the + # whole conversation let a later read overwrite an earlier read that reused + # the same id, which dropped the earlier skill from the ledger and logged a + # mismatch for a read that was correct. + skill_paths_by_id: dict[str, str] = {} for index, message in enumerate(messages): + if isinstance(message, AIMessage): + skill_paths_by_id = {} + for tool_call in message.tool_calls or []: + if _tool_call_name(tool_call) not in read_names: + continue + tool_call_id = _tool_call_id(tool_call) + raw_path = _tool_call_path(tool_call) + path = _normalize_under_root(raw_path, normalized_root) if raw_path else None + if tool_call_id and path and _is_skill_file(path): + skill_paths_by_id[tool_call_id] = path + continue if not isinstance(message, ToolMessage): continue if getattr(message, "status", "success") == "error": diff --git a/backend/tests/test_skill_context.py b/backend/tests/test_skill_context.py index c4e4c60d09..d10a9435dd 100644 --- a/backend/tests/test_skill_context.py +++ b/backend/tests/test_skill_context.py @@ -221,6 +221,98 @@ class TestExtractSkills: out = extract_skills(msgs, skills_root=_ROOT, read_tool_names=_READ) assert [e["name"] for e in out] == ["a", "b"] + def test_same_tool_call_id_reused_by_a_later_read_keeps_both_skills(self): + msgs = [ + _ai_read("call_0", "/mnt/skills/public/a/SKILL.md"), + ToolMessage( + content="A", + tool_call_id="call_0", + id="tm1", + additional_kwargs=_skill_metadata("/mnt/skills/public/a/SKILL.md", "A"), + ), + _ai_read("call_0", "/mnt/skills/custom/b/SKILL.md"), + ToolMessage( + content="B", + tool_call_id="call_0", + id="tm2", + additional_kwargs=_skill_metadata("/mnt/skills/custom/b/SKILL.md", "B"), + ), + ] + + out = extract_skills(msgs, skills_root=_ROOT, read_tool_names=_READ) + + assert [e["name"] for e in out] == ["a", "b"] + assert [e["path"] for e in out] == [ + "/mnt/skills/public/a/SKILL.md", + "/mnt/skills/custom/b/SKILL.md", + ] + + def test_same_tool_call_id_reused_does_not_warn_for_the_earlier_read(self, caplog): + msgs = [ + _ai_read("call_0", "/mnt/skills/public/a/SKILL.md"), + ToolMessage( + content="A", + tool_call_id="call_0", + id="tm1", + additional_kwargs=_skill_metadata("/mnt/skills/public/a/SKILL.md", "A"), + ), + _ai_read("call_0", "/mnt/skills/custom/b/SKILL.md"), + ToolMessage( + content="B", + tool_call_id="call_0", + id="tm2", + additional_kwargs=_skill_metadata("/mnt/skills/custom/b/SKILL.md", "B"), + ), + ] + + with caplog.at_level("WARNING", logger="deerflow.agents.middlewares.skill_context"): + out = extract_skills(msgs, skills_root=_ROOT, read_tool_names=_READ) + + assert len(out) == 2 + assert "mismatched skill read metadata" not in caplog.text + + def test_tool_result_whose_call_is_only_in_an_earlier_response_is_skipped(self, caplog): + # A tool-call-less AIMessage between the read and its late result must + # still close the response: the result cannot pair across it. + msgs = [ + _ai_read("call_0", "/mnt/skills/public/a/SKILL.md"), + AIMessage(content="thinking"), + ToolMessage( + content="A", + tool_call_id="call_0", + id="tm1", + additional_kwargs=_skill_metadata("/mnt/skills/public/a/SKILL.md", "A"), + ), + ] + + with caplog.at_level("WARNING", logger="deerflow.agents.middlewares.skill_context"): + out = extract_skills(msgs, skills_root=_ROOT, read_tool_names=_READ) + + assert out == [] + assert "mismatched skill read metadata" not in caplog.text + assert "missing skill read metadata" not in caplog.text + + def test_missing_metadata_warning_points_at_the_later_reads_path(self, caplog): + msgs = [ + _ai_read("call_0", "/mnt/skills/public/a/SKILL.md"), + ToolMessage( + content="A", + tool_call_id="call_0", + id="tm1", + additional_kwargs=_skill_metadata("/mnt/skills/public/a/SKILL.md", "A"), + ), + _ai_read("call_0", "/mnt/skills/custom/b/SKILL.md"), + ToolMessage(content="B", tool_call_id="call_0", id="tm2"), + ] + + with caplog.at_level("WARNING", logger="deerflow.agents.middlewares.skill_context"): + out = extract_skills(msgs, skills_root=_ROOT, read_tool_names=_READ) + + assert [e["name"] for e in out] == ["a"] + assert "missing skill read metadata" in caplog.text + assert "/mnt/skills/custom/b/SKILL.md" in caplog.text + assert "/mnt/skills/public/a/SKILL.md" not in caplog.text + def test_extract_skills_prefers_metadata_only_when_path_matches_read_call(self): msgs = [ _ai_read("r1", "/mnt/skills/public/data-analysis/SKILL.md"),