diff --git a/CHANGELOG.md b/CHANGELOG.md index a1768aa6..bf60e670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,13 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- Preserve explicit Claude and OpenCode read failure metadata in reconstructed + ATIF trajectories. Failed isolated SKILL.md reads and failed native Skill + calls lose credit; compound shell failures and shell reads whose path may + expand to several files (braces, globs, `$` or backticks) remain ambiguous + and retain prior credit in host and verifier grading. Unattributed errors do + not cancel successful reads, and shell reads count dash-prefixed filenames + after `--`. - Tier 1 pyproject dependency audits now inspect static target declarations rather than the evaluator environment. Exact direct pins (including optional groups) are queried without installation or build-script execution; diff --git a/src/skillevaluator/tier3/eval_core/atif_helpers.py b/src/skillevaluator/tier3/eval_core/atif_helpers.py index d6d0da1f..9b531780 100644 --- a/src/skillevaluator/tier3/eval_core/atif_helpers.py +++ b/src/skillevaluator/tier3/eval_core/atif_helpers.py @@ -17,7 +17,11 @@ from typing import Any from skillevaluator.evidence import evidence_ref_identity -from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import atif_content_has_text, atif_content_text +from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import ( + atif_content_has_text, + atif_content_text, + normalized_tool_call_is_error, +) from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import ( iter_normalized_tool_calls as iter_tool_calls, ) @@ -53,11 +57,15 @@ def get_all_tool_calls(traj: dict[str, Any]) -> list[dict[str, Any]]: def get_skill_tool_calls(traj: dict[str, Any]) -> list[str]: """Get skill names from Claude Code's native ``Skill`` tool invocations.""" skills: list[str] = [] - for tc in get_all_tool_calls(traj): - if tc["fn"].lower() == "skill": - name = tc["args"].get("skill", tc["args"].get("name", "")) - if name: - skills.append(str(name)) + for step, tc in iter_tool_calls(traj): + if (tc.get("function_name") or "").lower() != "skill": + continue + if normalized_tool_call_is_error(step, tc): + continue + args = tc.get("arguments") or {} + name = args.get("skill", args.get("name", "")) + if name: + skills.append(str(name)) return skills @@ -1054,6 +1062,8 @@ def extract_tool_calls_as_dicts(traj: dict[str, Any]) -> list[dict[str, Any]]: "action_input": tc.get("arguments") or {}, "observation": _tool_call_observation(step, tc), } + if normalized_tool_call_is_error(step, tc): + call["is_error"] = True if status := tc.get("_atif_normalization_status"): call["normalization_status"] = status if status := tc.get("_atif_observation_status"): diff --git a/src/skillevaluator/tier3/eval_core/checks.py b/src/skillevaluator/tier3/eval_core/checks.py index 4ad180fc..aa21ec94 100644 --- a/src/skillevaluator/tier3/eval_core/checks.py +++ b/src/skillevaluator/tier3/eval_core/checks.py @@ -640,6 +640,38 @@ def _command_reads_skill_md_arg(command: list[str], cmd_idx: int, assignments: d return False +# Brace, glob, parameter and command expansion can turn one shell word into +# several files. Quotes are gone after tokenizing, so any operand holding one of +# these characters is treated as possibly naming more than one file. +_SHELL_EXPANSION_CHARS = frozenset("{*?[$`") + + +def _failed_single_skill_read(call): + """A shell failure only proves the skill read failed for an isolated read. + + Compound commands, multi-file reads and operands that the shell may expand + into several files can print SKILL.md successfully before another file or + operation fails. + """ + if call.get("is_error") is not True: + return False + tokens = _shell_tokens(_command_text(call)) + if not tokens or any(token in _SHELL_SEPARATORS for token in tokens): + return False + command = tokens[0].rsplit("/", 1)[-1].lower() + if command not in {"cat", "nl", "bat", "less", "more"}: + return False + files = [] + options_ended = False + for arg in tokens[1:]: + arg = arg.removeprefix(_QUOTED_SYNTAX_MARK) + if not options_ended and arg == "--": + options_ended = True + elif options_ended or arg == "-" or not arg.startswith("-"): + files.append(arg) + return len(files) == 1 and not _SHELL_EXPANSION_CHARS.intersection(files[0]) and _skill_md_arg(files[0], {}) + + def _cmd_reads_skill_md(cmd) -> bool: """True if a shell command reads a SKILL.md via a file-view utility. @@ -2018,7 +2050,7 @@ def check_activation( } # Check 2: read_file / read with SKILL.md in path - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] for call in read_calls: path_arg = _extract_path(call) if "SKILL.md" not in path_arg: @@ -2037,7 +2069,7 @@ def check_activation( } # Check 3: shell read of SKILL.md (cat/sed/head/...) - exec_calls = [tc for tc in tool_calls if _is_execution_action(str(tc["action"]))] + exec_calls = [tc for tc in tool_calls if _is_execution_action(str(tc["action"])) and not _failed_single_skill_read(tc)] for call in exec_calls: cmd = _command_text(call) if _cmd_reads_skill_md(cmd): @@ -2073,7 +2105,10 @@ def check_activation( has_skill_read_evidence = ("read" in action.lower() and "SKILL.md" in str(tc.get("action_input", ""))) or ( _is_execution_action(action) and _cmd_reads_skill_md(cmd) ) - if not has_skill_read_evidence: + if not has_skill_read_evidence or ( + tc.get("is_error") is True + and (not _is_execution_action(action) or _failed_single_skill_read(tc)) + ): continue obs = str(tc.get("observation", "")).lower() if "skill.md" in obs: @@ -5107,7 +5142,10 @@ def check_workflow_order( cmd = _command_text(call) if ("read" in action and "SKILL.md" in args_str) or (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): - sequence.append("read_skill") + if call.get("is_error") is not True or ( + _is_execution_action(action) and not _failed_single_skill_read(call) + ): + sequence.append("read_skill") elif _is_execution_action(action) and cmd and "--help" not in cmd and "which " not in cmd: sequence.append("execution") @@ -5365,7 +5403,7 @@ def check_routing( ) -> dict[str, Any]: """Check the agent read only expected/allowed workspace skill docs.""" unsupported_native_codex_call = _has_unsupported_native_codex_call(tool_calls) - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] skills_read: list[str] = [] wrong_skills: list[str] = [] @@ -5398,7 +5436,7 @@ def check_routing( for call in tool_calls: action = call["action"].lower() cmd = _command_text(call) - if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): + if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)) or _failed_single_skill_read(call): continue skills_read.append(cmd) diff --git a/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py b/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py index 9369bcc5..a8c1af9c 100644 --- a/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py +++ b/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py @@ -101,6 +101,22 @@ def normalized_tool_call_observation(step: dict[str, Any], tool_call: dict[str, ) +def normalized_tool_call_is_error(step: dict[str, Any], tool_call: dict[str, Any]) -> bool: + """Preserve explicit failure metadata only for an attributable tool result.""" + if tool_call.get("_atif_observation_status") not in (None, MAPPED_OUTER_EXEC_OBSERVATION): + return False + tool_call_id = tool_call.get("tool_call_id") + for result in (step.get("observation") or {}).get("results") or []: + source_call_id = result.get("source_call_id") + # Unscoped observations cannot prove that this particular call failed. + if not tool_call_id or source_call_id != tool_call_id: + continue + extra = result.get("extra") or {} + if isinstance(extra, dict) and extra.get("tool_result_is_error") is True: + return True + return False + + def normalized_tool_call_wrapper_observation(step: dict[str, Any], tool_call: dict[str, Any]) -> str: """Return an unattributed wrapper observation once for security scanning.""" if tool_call.get("_atif_observation_status") not in ( diff --git a/src/skillevaluator/tier3/eval_core/log_converters.py b/src/skillevaluator/tier3/eval_core/log_converters.py index 4f4812d4..2effe9db 100644 --- a/src/skillevaluator/tier3/eval_core/log_converters.py +++ b/src/skillevaluator/tier3/eval_core/log_converters.py @@ -54,7 +54,9 @@ def _stringify_tool_result_content(content: Any) -> str: return str(content) -def _attach_tool_result(steps: list[dict[str, Any]], tool_use_id: str, content_text: str) -> None: +def _attach_tool_result( + steps: list[dict[str, Any]], tool_use_id: str, content_text: str, *, is_error: bool = False +) -> None: for step in reversed(steps): if step.get("source") != "agent": continue @@ -62,12 +64,11 @@ def _attach_tool_result(steps: list[dict[str, Any]], tool_use_id: str, content_t if tc.get("tool_call_id") == tool_use_id: step.setdefault("observation", {"results": []}) step["observation"].setdefault("results", []) - step["observation"]["results"].append( - { - "source_call_id": tool_use_id, - "content": content_text, - } - ) + result: dict[str, Any] = {"source_call_id": tool_use_id, "content": content_text} + if is_error: + # Match Harbor's native Claude Code ATIF result metadata. + result["extra"] = {"tool_result_is_error": True} + step["observation"]["results"].append(result) return @@ -158,7 +159,7 @@ def synthetic_trajectory_from_claude_stream_jsonl(text: str) -> dict[str, Any] | tid = str(block.get("tool_use_id") or "") res_text = _stringify_tool_result_content(block.get("content")) if tid: - _attach_tool_result(steps, tid, res_text) + _attach_tool_result(steps, tid, res_text, is_error=block.get("is_error") is True) elif et == "result": res = evt.get("result") @@ -564,13 +565,12 @@ def synthetic_trajectory_from_opencode_json(text: str) -> dict[str, Any] | None: "observation": {"results": []}, } observation_text = _opencode_tool_observation(state) - if observation_text: - step["observation"]["results"].append( - { - "source_call_id": call_id, - "content": observation_text, - } - ) + failed_read = status == "error" and function_name.lower() in {"read", "read_file"} + if observation_text or failed_read: + result = {"source_call_id": call_id, "content": observation_text} + if failed_read: + result["extra"] = {"tool_result_is_error": True} + step["observation"]["results"].append(result) steps.append(step) if not saw_content or not steps: diff --git a/src/skillevaluator/tier3/harbor/templates/eval.py b/src/skillevaluator/tier3/harbor/templates/eval.py index 2c3087f4..c7538b7c 100644 --- a/src/skillevaluator/tier3/harbor/templates/eval.py +++ b/src/skillevaluator/tier3/harbor/templates/eval.py @@ -77,6 +77,7 @@ def load_trajectory_with_fallback(trajectory_path, logs_dir=None): atif_content_has_text, atif_content_text, iter_normalized_tool_calls, + normalized_tool_call_is_error, normalized_tool_call_observation, normalized_tool_call_wrapper_observation, ) @@ -88,6 +89,7 @@ def load_trajectory_with_fallback(trajectory_path, logs_dir=None): atif_content_has_text, atif_content_text, iter_normalized_tool_calls, + normalized_tool_call_is_error, normalized_tool_call_observation, normalized_tool_call_wrapper_observation, ) @@ -403,11 +405,15 @@ def get_all_tool_calls(traj): def get_skill_tool_calls(traj): skills = [] - for tc in get_all_tool_calls(traj): - if tc["fn"].lower() == "skill": - name = tc["args"].get("skill", tc["args"].get("name", "")) - if name: - skills.append(str(name)) + for step, tc in iter_tool_calls(traj): + if (tc.get("function_name") or "").lower() != "skill": + continue + if normalized_tool_call_is_error(step, tc): + continue + args = tc.get("arguments") or {} + name = args.get("skill", args.get("name", "")) + if name: + skills.append(str(name)) return skills @@ -458,6 +464,8 @@ def extract_tool_calls_as_dicts(traj): "action_input": tc.get("arguments") or {}, "observation": _tool_call_observation(step, tc), } + if normalized_tool_call_is_error(step, tc): + call["is_error"] = True if status := tc.get("_atif_normalization_status"): call["normalization_status"] = status if status := tc.get("_atif_observation_status"): @@ -3260,6 +3268,38 @@ def _command_reads_skill_md_arg(command, cmd_idx, assignments): return False +# Brace, glob, parameter and command expansion can turn one shell word into +# several files. Quotes are gone after tokenizing, so any operand holding one of +# these characters is treated as possibly naming more than one file. +_SHELL_EXPANSION_CHARS = frozenset("{*?[$`") + + +def _failed_single_skill_read(call): + """A shell failure only proves the skill read failed for an isolated read. + + Compound commands, multi-file reads and operands that the shell may expand + into several files can print SKILL.md successfully before another file or + operation fails. + """ + if call.get("is_error") is not True: + return False + tokens = _shell_tokens(_command_text(call)) + if not tokens or any(token in _SHELL_SEPARATORS for token in tokens): + return False + command = tokens[0].rsplit("/", 1)[-1].lower() + if command not in {"cat", "nl", "bat", "less", "more"}: + return False + files = [] + options_ended = False + for arg in tokens[1:]: + arg = arg.removeprefix(_QUOTED_SYNTAX_MARK) + if not options_ended and arg == "--": + options_ended = True + elif options_ended or arg == "-" or not arg.startswith("-"): + files.append(arg) + return len(files) == 1 and not _SHELL_EXPANSION_CHARS.intersection(files[0]) and _skill_md_arg(files[0], {}) + + def _cmd_reads_skill_md(cmd) -> bool: """True if a shell command reads a SKILL.md via a file-view utility. @@ -4574,7 +4614,7 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab "reason": reason, "details": {**_skill_match_details(expected_skill, acceptable_skills), **match}, } - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] for call in read_calls: path_arg = _extract_path(call) if "SKILL.md" not in path_arg: @@ -4592,7 +4632,7 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab "details": {**_skill_match_details(expected_skill, acceptable_skills), **match, "path": path_arg}, } for call in tool_calls: - if _is_execution_action(call["action"]): + if _is_execution_action(call["action"]) and not _failed_single_skill_read(call): cmd = _command_text(call) if _cmd_reads_skill_md(cmd): match = _classify_skill_match(cmd, expected_skill, acceptable_skills, fuzzy=True) @@ -4624,7 +4664,10 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab has_skill_read_evidence = ("read" in action.lower() and "SKILL.md" in str(tc.get("action_input", ""))) or ( _is_execution_action(action) and _cmd_reads_skill_md(cmd) ) - if not has_skill_read_evidence: + if not has_skill_read_evidence or ( + tc.get("is_error") is True + and (not _is_execution_action(action) or _failed_single_skill_read(tc)) + ): continue obs = str(tc.get("observation", "")).lower() if "skill.md" in obs: @@ -7641,7 +7684,10 @@ def check_workflow_order(tool_calls, skill_tool_names=None, expected_skill=None) args_str = str(call.get("action_input", "")) cmd = _command_text(call) if ("read" in action and "SKILL.md" in args_str) or (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): - sequence.append("read_skill") + if call.get("is_error") is not True or ( + _is_execution_action(action) and not _failed_single_skill_read(call) + ): + sequence.append("read_skill") elif _is_execution_action(action) and cmd and "--help" not in cmd and "which " not in cmd: sequence.append("execution") if not sequence: @@ -7724,7 +7770,7 @@ def check_routing( acceptable_skills=None, ): unsupported_native_codex_call = _has_unsupported_native_codex_call(tool_calls) - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] skills_read, wrong_skills = [], [] matched_expected = False matched_alternate = False @@ -7752,7 +7798,7 @@ def check_routing( for call in tool_calls: action = call["action"].lower() cmd = _command_text(call) - if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): + if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)) or _failed_single_skill_read(call): continue skills_read.append(cmd) skill_name = _skill_name_from_ref(cmd) diff --git a/tests/tier3/test_failed_skill_reads.py b/tests/tier3/test_failed_skill_reads.py new file mode 100644 index 00000000..e69ea0de --- /dev/null +++ b/tests/tier3/test_failed_skill_reads.py @@ -0,0 +1,390 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Failed tool results cannot prove a successful SKILL.md read.""" + +from __future__ import annotations + +import json +import shlex +import subprocess +from pathlib import Path +from types import ModuleType + +import pytest + +from skillevaluator.tier3.eval_core import atif_helpers, checks +from skillevaluator.tier3.eval_core.log_converters import synthetic_trajectory_from_claude_stream_jsonl + + +@pytest.fixture(params=("host", "verifier")) +def grader(request: pytest.FixtureRequest, harbor_eval_template: ModuleType) -> tuple[ModuleType, ModuleType]: + if request.param == "host": + return atif_helpers, checks + return harbor_eval_template, harbor_eval_template + + +def _claude_tool_result(*, name: str, arguments: dict, is_error: bool, content: str) -> dict: + events = [ + { + "type": "assistant", + "message": { + "content": [ + { + "type": "tool_use", + "id": "read-1", + "name": name, + "input": arguments, + } + ] + }, + }, + { + "type": "user", + "message": { + "content": [ + { + "type": "tool_result", + "tool_use_id": "read-1", + "is_error": is_error, + "content": content, + } + ] + }, + }, + ] + trajectory = synthetic_trajectory_from_claude_stream_jsonl("\n".join(json.dumps(event) for event in events)) + assert trajectory is not None + return trajectory + + +def _claude_read(*, is_error: bool, content: str) -> dict: + return _claude_tool_result( + name="Read", + arguments={"file_path": "/skills/demo-skill/SKILL.md"}, + is_error=is_error, + content=content, + ) + + +def test_claude_failed_read_cannot_receive_perfect_execution_credit(grader: tuple[ModuleType, ModuleType]) -> None: + extractor, scorer = grader + trajectory = _claude_read(is_error=True, content="Error: File does not exist: /skills/demo-skill/SKILL.md") + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + execution = scorer.score_skill_execution(calls, "demo-skill") + assert execution["details"]["activation"]["score"] == 0.0 + assert execution["details"]["workflow_order"]["score"] == 0.0 + assert execution["score"] < 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 0.0 + + +def test_successful_skill_content_can_discuss_errors(grader: tuple[ModuleType, ModuleType]) -> None: + extractor, scorer = grader + calls = extractor.extract_tool_calls_as_dicts( + _claude_read(is_error=False, content="# Demo skill\nError: File does not exist. Handle this by retrying.") + ) + + assert scorer.score_skill_execution(calls, "demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize("action", ("Read", "Bash")) +def test_failed_read_does_not_satisfy_workflow_before_execution( + grader: tuple[ModuleType, ModuleType], + action: str, +) -> None: + _, scorer = grader + read = { + "action": action, + "action_input": {"file_path": "/skills/demo-skill/SKILL.md"} + if action == "Read" + else {"command": "cat /skills/demo-skill/SKILL.md"}, + "observation": "Unable to load /skills/demo-skill/SKILL.md", + "is_error": True, + } + execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}, "observation": "ok"} + + assert scorer.check_activation([read], "demo-skill")["score"] == 0.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 0.0 + assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 0.0 + retry = {**read, "observation": "# Demo skill", "is_error": False} + assert scorer.check_workflow_order([read, retry, execute], expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing([read, retry], "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize( + "command", + ( + "cat /skills/demo-skill/SKILL.md && ls /skills/demo-skill/scripts/", + "cat /skills/demo-skill/SKILL.md /skills/demo-skill/reference.md", + "cat /skills/demo-skill/SKILL.md -", + ), +) +def test_failed_compound_shell_read_keeps_credit(grader, command) -> None: + _, scorer = grader + read = {"action": "Bash", "action_input": {"command": command}, "observation": "# Demo skill", "is_error": True} + execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}} + assert scorer.check_activation([read], "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 1.0 + + +def test_failed_native_skill_call_is_not_activation(grader) -> None: + extractor, scorer = grader + trajectory = { + "steps": [ + { + "source": "agent", + "message": "", + "tool_calls": [{"tool_call_id": "s1", "function_name": "Skill", "arguments": {"skill": "demo-skill"}}], + "observation": { + "results": [ + { + "source_call_id": "s1", + "content": "Skill not found", + "extra": {"tool_result_is_error": True}, + } + ] + }, + } + ] + } + calls = extractor.extract_tool_calls_as_dicts(trajectory) + assert extractor.get_skill_tool_calls(trajectory) == [] + assert scorer.check_activation(calls, "demo-skill", skill_tool_names=[])["score"] == 0.0 + assert scorer.check_workflow_order(calls, skill_tool_names=[], expected_skill="demo-skill")["score"] == 0.0 + assert scorer.check_routing(calls, "demo-skill", skill_tool_names=[])["score"] == 0.0 + + +def test_opencode_failed_read_is_marked_as_error(grader) -> None: + from skillevaluator.tier3.eval_core.log_converters import synthetic_trajectory_from_opencode_json + + extractor, scorer = grader + event = { + "type": "tool_use", + "part": { + "tool": "read", + "callID": "failed", + "state": { + "status": "error", + "input": {"filePath": "/skills/demo-skill/SKILL.md"}, + "error": {"message": "File not found"}, + }, + }, + } + trajectory = synthetic_trajectory_from_opencode_json(json.dumps(event)) + assert trajectory is not None + calls = extractor.extract_tool_calls_as_dicts(trajectory) + assert calls[0]["is_error"] is True + assert scorer.check_activation(calls, "demo-skill")["score"] == 0.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 0.0 + + +def test_native_atif_error_is_matched_to_its_own_call(grader: tuple[ModuleType, ModuleType]) -> None: + from harbor.models.trajectories.trajectory import Trajectory + + extractor, scorer = grader + trajectory = Trajectory.model_validate( + { + "schema_version": "ATIF-v1.7", + "agent": {"name": "claude-code", "version": "test"}, + "steps": [ + { + "step_id": 1, + "source": "agent", + "message": "", + "tool_calls": [ + { + "tool_call_id": "wrong", + "function_name": "Read", + "arguments": {"file_path": "/skills/wrong/SKILL.md"}, + }, + { + "tool_call_id": "right", + "function_name": "Read", + "arguments": {"file_path": "/skills/demo-skill/SKILL.md"}, + }, + ], + "observation": { + "results": [ + { + "source_call_id": "right", + "content": "# Demo skill", + "extra": {"tool_result_is_error": False}, + }, + { + "source_call_id": "wrong", + "content": "Missing file", + "extra": {"tool_result_is_error": True}, + }, + ] + }, + } + ], + } + ).to_json_dict() + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + assert not calls[1].get("is_error") + assert scorer.score_skill_execution(calls, "demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize("action", ("Read", "Skill")) +def test_unrelated_error_does_not_cancel_successful_skill_read( + grader: tuple[ModuleType, ModuleType], action: str +) -> None: + from harbor.models.trajectories.trajectory import Trajectory + + extractor, scorer = grader + trajectory = Trajectory.model_validate( + { + "schema_version": "ATIF-v1.7", + "agent": {"name": "claude-code", "version": "test"}, + "steps": [ + { + "step_id": 1, + "source": "agent", + "message": "", + "tool_calls": [ + { + "tool_call_id": "read-1", + "function_name": action, + "arguments": {"file_path": "/skills/demo-skill/SKILL.md"} + if action == "Read" + else {"skill": "demo-skill"}, + } + ], + "observation": { + "results": [ + { + "source_call_id": "read-1", + "content": "# Demo skill", + "extra": {"tool_result_is_error": False}, + }, + { + "source_call_id": None, + "content": "Unrelated system action failed", + "extra": {"tool_result_is_error": True}, + }, + ] + }, + } + ], + } + ).to_json_dict() + calls = extractor.extract_tool_calls_as_dicts(trajectory) + names = extractor.get_skill_tool_calls(trajectory) + + assert not calls[0].get("is_error") + assert names == (["demo-skill"] if action == "Skill" else []) + assert scorer.score_skill_execution(calls, "demo-skill", skill_tool_names=names)["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill", skill_tool_names=names)["score"] == 1.0 + + +@pytest.mark.parametrize("delimiter", ("--", "'--'")) +def test_failed_cat_with_dash_prefixed_second_file_keeps_read_credit( + grader: tuple[ModuleType, ModuleType], tmp_path: Path, delimiter: str +) -> None: + extractor, scorer = grader + skill = tmp_path / "demo-skill" / "SKILL.md" + skill.parent.mkdir() + skill.write_text("# Demo skill\nActual instructions\n") + command = f"cat {delimiter} {shlex.quote(str(skill))} -missing-reference.md" + result = subprocess.run(["bash", "-c", command], cwd=tmp_path, capture_output=True, text=True) + assert result.returncode == 1 + assert result.stdout == skill.read_text() + assert "-missing-reference.md" in result.stderr + trajectory = _claude_tool_result( + name="Bash", + arguments={"command": command}, + is_error=True, + content=result.stdout + result.stderr, + ) + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + assert scorer.check_activation(calls, "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order(calls, expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +def test_failed_brace_expanded_cat_keeps_read_credit(grader: tuple[ModuleType, ModuleType], tmp_path: Path) -> None: + extractor, scorer = grader + skill = tmp_path / "skills" / "demo-skill" / "SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text("---\nname: demo-skill\n---\n# Demo skill\n", encoding="utf-8") + command = f"cat {shlex.quote(str(tmp_path / 'skills'))}/{{demo-skill,missing}}/SKILL.md" + result = subprocess.run(["bash", "-c", command], cwd=tmp_path, capture_output=True, text=True, encoding="utf-8") + assert result.returncode == 1 + assert result.stdout == skill.read_text(encoding="utf-8") + assert "missing/SKILL.md" in result.stderr + trajectory = _claude_tool_result( + name="Bash", + arguments={"command": command}, + is_error=True, + content=result.stdout + result.stderr, + ) + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + assert scorer.check_activation(calls, "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order(calls, expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize( + "command", + ( + "cat /skills/*/SKILL.md", + "cat /skills/[a-z]*/SKILL.md", + "cat /skills/demo-skill?/SKILL.md", + "cat $SKILLS_DIR/SKILL.md", + "cat `pwd`/SKILL.md", + ), +) +def test_failed_cat_with_expanding_operand_keeps_read_credit(grader, command: str) -> None: + _, scorer = grader + read = { + "action": "Bash", + "action_input": {"command": command}, + "observation": "---\nname: demo-skill\n---\n# Demo skill\ncat: /skills/other/SKILL.md: Is a directory", + "is_error": True, + } + execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}} + assert scorer.check_activation([read], "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 1.0 + + +def test_failed_cat_of_home_relative_skill_loses_credit(grader) -> None: + # Tilde expansion always yields one path, so this is still an isolated read. + _, scorer = grader + read = { + "action": "Bash", + "action_input": {"command": "cat ~/.claude/skills/demo-skill/SKILL.md"}, + "observation": "cat: /root/.claude/skills/demo-skill/SKILL.md: No such file or directory", + "is_error": True, + } + assert scorer.check_activation([read], "demo-skill")["score"] == 0.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 0.0 + + +@pytest.mark.parametrize("options", ("--", "'--'", "-n", "-n --")) +@pytest.mark.parametrize("is_error", (False, True)) +def test_single_cat_read_with_options(grader: tuple[ModuleType, ModuleType], options: str, is_error: bool) -> None: + extractor, scorer = grader + trajectory = _claude_tool_result( + name="Bash", + arguments={"command": f"cat {options} /skills/demo-skill/SKILL.md"}, + is_error=is_error, + content="File not found" if is_error else "# Demo skill", + ) + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert scorer.check_activation(calls, "demo-skill")["score"] == (0.0 if is_error else 0.75) + assert scorer.check_workflow_order(calls, expected_skill="demo-skill")["score"] == (0.0 if is_error else 1.0) + assert scorer.check_routing(calls, "demo-skill")["score"] == (0.0 if is_error else 1.0)