From 16b4fcd968b8100984cf4ba8fba119bcac844d2e Mon Sep 17 00:00:00 2001 From: Hyeonsang Cho Date: Thu, 8 Oct 2026 20:44:52 +0900 Subject: [PATCH] fix(skills): keep a slash skill activation when the model call is retried (#6506) * fix(skills): keep a slash skill activation when the model call is retried SkillActivationMiddleware records the activated slash message in the run context before it calls the model. LLMErrorHandlingMiddleware sits outside it and retries a failed (429/503/timeout) or empty call by running this wrap again on the same request; the retry found the run key already set and went out without the SKILL.md body, while the slash source it had published kept the skill's tool restrictions in force. Record the activation itself next to the run key and replay it while nothing has answered the slash message yet: a retry sees no AI message after the target, a tool-loop follow-up call does. The replay carries the same reminder and usage stamp without re-reading the skill or repeating the audit event and usage record, which now sit with the other once-per-activation side effects. Two existing tests sent the same unanswered request twice to model a follow-up call; that is the retry shape, so their second call now includes the first response, as the real graph does. * docs(changelog): reference #6506 --------- Co-authored-by: Willem Jiang --- CHANGELOG.md | 8 ++ CHANGELOG_zh.md | 4 + .../deerflow/agents/middlewares/AGENTS.md | 2 +- .../skill_activation_middleware.py | 109 +++++++++------ .../deerflow/runtime/secret_context.py | 6 +- backend/tests/test_inline_skill_references.py | 12 +- backend/tests/test_slash_skills.py | 127 +++++++++++++++++- 7 files changed, 217 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1abd166e4f..1c5cb0f7a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -804,6 +804,13 @@ This release closes that milestone with **439 merged pull requests**. ### Fixed +- **skills:** A `/skill-name` activation now survives a retried model call. The + activation was marked as done before the model was called, so when the call + failed (rate limit, overload, timeout) or came back empty and was retried, the + retry went out without the `SKILL.md` body while the skill's tool restrictions + still applied. The retry now carries the same reminder as the first attempt, + without re-reading the skill or recording a second activation, and the retried + response keeps the skill-usage record. ([#6506]) - **skills:** Skill changes made through one Gateway replica now reach the skill list in every other replica's system prompt. `SkillStorage` rescans disk on every call, but the prompt layer caches the enabled-skills list and @@ -9221,3 +9228,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6450]: https://github.com/bytedance/deer-flow/pull/6450 [#6481]: https://github.com/bytedance/deer-flow/pull/6481 [#6495]: https://github.com/bytedance/deer-flow/pull/6495 +[#6506]: https://github.com/bytedance/deer-flow/pull/6506 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index f49f323323..08cd6095c0 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -694,6 +694,9 @@ ### 修复 +- **技能：** `/技能名` 激活现在在模型调用重试时不再丢失。此前激活在调用模型之前就被标记为已完成，因此调用失败 + （限流、过载、超时）或返回空响应而重试时，重试请求不再包含 `SKILL.md` 正文，而该技能的工具限制仍然生效。现在重试 + 会携带与首次尝试相同的激活内容，不会重新读取技能或重复记录激活，重试得到的响应也保留技能使用记录。([#6506]) - **持久化：** `scripts/migrate_user_isolation.py` 现在会把每个 legacy 线程移动到其所属用户下。此前脚本在 `{base_dir}/deer-flow.db` 中查找线程归属，而 DeerFlow 从不创建这个文件（数据库是 `{sqlite_dir}/deerflow.db` 或 PostgreSQL），因此归属映射始终为空，所有 legacy 线程都被移到 `users/default/`。现在归属从 `config.yaml` @@ -7528,3 +7531,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#6448]: https://github.com/bytedance/deer-flow/pull/6448 [#6450]: https://github.com/bytedance/deer-flow/pull/6450 [#6481]: https://github.com/bytedance/deer-flow/pull/6481 +[#6506]: https://github.com/bytedance/deer-flow/pull/6506 diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index 8422f399b6..2af0ee0179 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -103,7 +103,7 @@ Before changing a later authorization phase, read the [authorization RFC](../../ **Lead-only middlewares** (`build_middlewares`, appended after the base): 17. **DynamicContextMiddleware** - Injects date and optional memory outside the static prompt. Opt-out removes its frozen server memory but retains date/user messages. Date follows server-local time unless `DEER_FLOW_DATE_TIMEZONE` names an IANA zone (invalid values fall back locally). -18. **SkillActivationMiddleware** - Detects strict `/skill-name task` syntax on the latest real user message, resolves only enabled and runtime-allowed skills, injects the `SKILL.md` body as hidden current-turn context, and records a `middleware:skill_activation` audit event +18. **SkillActivationMiddleware** - Detects strict `/skill-name task` syntax on the latest real user message, resolves only enabled and runtime-allowed skills, injects the `SKILL.md` body as hidden current-turn context, and records a `middleware:skill_activation` audit event; a retried activation call replays the recorded reminder without re-auditing 19. **SkillToolPolicyMiddleware** - Applies `allowed-tools` only after real activation; passive enabled skills and a custom agent's configured skill allowlist do not clamp the lead toolset. A run-scoped slash activation is authoritative and suppresses `skill_context` as a policy source, so reading another skill cannot widen the explicit skill's tools; without slash activation, skills captured after configured `read_file` loads retain the existing union semantics. The middleware filters model-visible schemas and blocks unauthorized execution, resolving canonical paths against the live enabled/agent-allowed registry on every model call, then stores a versioned, JSON-safe, middleware-token-bound decision signed by policy source plus active paths in run context for the resulting tool calls to reuse. The next model call always refreshes it, and malformed, foreign, stale, or unmatched decisions fall back to live resolution. `tool_search` and `describe_skill` remain framework-safe discovery tools under a restrictive policy; they may reveal or promote metadata, but a deferred business tool must still be declared by the active policy before its schema or execution can survive the policy middleware. The decision's owner token is authorization-sensitive, so its reserved context key is owned by `runtime.secret_context` and included in `REDACTED_CONTEXT_KEYS` for observable and persisted context copies. Registry load failures and a non-empty active set with no authorized skill fail closed to framework-safe tools; an individual stale path is skipped only when at least one valid active skill remains. This is best-effort behavioral scoping rather than a hard security boundary: alternate loads such as `bash cat` are not captured, and bounded autonomous `skill_context` can evict old entries. `task` is not framework-exempt, so a restricted skill cannot delegate around its policy. The middleware must remain immediately after `SkillActivationMiddleware` (which publishes the slash source through `runtime.secret_context`'s public path helpers authenticated by a required token shared only within the assembled middleware chain) and immediately before `DurableContextMiddleware`; assembly and compiled-graph tests pin ordering, token sharing, schema filtering, and execution blocking. 20. **DurableContextMiddleware** - Before compaction, captures task dispatches/result summaries in `ThreadState.delegations` and loaded skill name/path/description references (never bodies) in `ThreadState.skill_context`; projects them into each model request. Static authority rules are injected as a `SystemMessage`; untrusted field values (`summary_text`, delegation results, skill descriptions) are injected separately as a hidden `HumanMessage` data block so compressed history, delegated work, and which skills are active stay visible without being stored as `messages` or promoted to system-role instructions. `build_subagent_runtime_middlewares` also attaches this middleware immediately before subagent summarization so a compacted `summary_text` is projected ahead of a preserved assistant/tool tail instead of leaving strict providers with an assistant-first request. See [tool artifacts](TOOL_ARTIFACTS.md) for projection, retry limits and delegation scope. 21. **SummarizationMiddleware** - *(optional)* Compacts near token limits. Fraction trigger/keep profiles belong to the active run model; an explicit summary model only generates. Memory flush/manual compaction follow runtime/checkpoint agent policy. Preserve the latest user request and DynamicContext reminders; a backward cutoff may retain old AI/tool turns or make first-turn compaction a no-op. diff --git a/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py index 40d646e165..d58f770d0f 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py @@ -64,13 +64,15 @@ _REGISTRY_LOAD_FAILED = object() # injection set is recomputed every model call, but a slash-activated skill must # stay bound for the rest of the run — the model's tool loop issues many model # calls after the single activation call (#3861 semantics). -# _SLASH_SKILL_ACTIVATION_RUN_KEY: identity of the slash message already activated -# in this run, so the reminder injection + skill disk read + "activate" audit event -# fire once per user slash command instead of on every model call. The reminder is -# added via request.override(messages=...) for a single model call and never -# persisted to graph state, so the 2nd..Nth model call of a turn rebuilds -# request.messages from state without it — the run context is the only signal that -# survives the tool loop. All three live in secret_context so they are covered by +# _SLASH_SKILL_ACTIVATION_RUN_KEY: the slash message already activated in this run +# and the activation it produced (_RecordedActivation), so the skill disk read + +# "activate" audit event + usage record fire once per user slash command instead of +# on every model call. The reminder is added via request.override(messages=...) for +# a single model call and never persisted to graph state, so the 2nd..Nth model call +# of a turn rebuilds request.messages from state without it — the run context is +# the only signal that survives the tool loop. A retry of the activation call +# (nothing has answered the slash message yet) replays the recorded reminder rather +# than dropping it. All three live in secret_context so they are covered by # REDACTED_CONTEXT_KEYS in one place. @@ -91,6 +93,15 @@ class _Activation: class _ActivationResolution: activation: _Activation | None = None failure_message: str | None = None + # A retry of the call that already activated: re-inject the reminder, but the + # once-per-activation side effects (audit, usage record) already happened. + replayed: bool = False + + +@dataclass(frozen=True, slots=True) +class _RecordedActivation: + run_key: str + activation: _Activation def is_slash_skill_activation_reminder(message: object) -> bool: @@ -392,8 +403,8 @@ Follow this skill before choosing a general workflow. Load supporting resources return context if isinstance(context, dict) else None @staticmethod - def _already_activated(run_context: dict | None, run_key: str) -> bool: - """Whether ``run_key`` was already recorded as activated earlier in this run. + def _recorded_activation(run_context: dict | None, run_key: str) -> _Activation | None: + """The activation ``run_key`` already produced earlier in this run, if any. Sibling to ``_has_existing_activation_for_target``: that helper catches an activation reminder still present in the scanned ``messages`` window; this @@ -402,9 +413,24 @@ Follow this skill before choosing a general workflow. Load supporting resources ``_SLASH_SKILL_ACTIVATION_RUN_KEY``). ``run_key`` is computed once by the caller (``_find_activation_target``) and reused as-is at the write site in ``_prepare_model_request``, so the same key is always used to check and to - record — this helper only ever checks membership, never computes the key. + record — this helper only ever looks the key up, never computes it. """ - return isinstance(run_context, dict) and run_context.get(_SLASH_SKILL_ACTIVATION_RUN_KEY) == run_key + recorded = run_context.get(_SLASH_SKILL_ACTIVATION_RUN_KEY) if isinstance(run_context, dict) else None + if isinstance(recorded, _RecordedActivation) and recorded.run_key == run_key: + return recorded.activation + return None + + @staticmethod + def _target_answered(messages: list, target_index: int) -> bool: + """Whether a model response to the target already reached graph state. + + Tells the tool loop's follow-up calls apart from a retry of the activation + call. LLMErrorHandlingMiddleware sits outside this middleware and retries a + failed or empty call by running this wrap again on the same request, so the + retry sees no AI message after the target; a follow-up call sees the + response the activation call produced. + """ + return any(isinstance(message, AIMessage) for message in messages[target_index + 1 :]) def _find_activation_target(self, messages: list, *, run_context: dict | None = None, activation_decisions: dict[str, bool] | None = None) -> tuple[int, HumanMessage, _ActivationResolution, str] | None: if not messages: @@ -422,13 +448,19 @@ Follow this skill before choosing a general workflow. Load supporting resources # This exact slash message may have already activated earlier in the run. # The message scan above cannot catch it because the reminder lives only in # a per-call request override, never in state — the run context is the - # durable signal (see _already_activated / _SLASH_SKILL_ACTIVATION_RUN_KEY). - # Skipping here avoids the redundant skill disk read, reminder re-injection, - # and duplicate "activate" audit. run_key is computed once here and threaded + # durable signal (see _recorded_activation / _SLASH_SKILL_ACTIVATION_RUN_KEY). + # Once the activation call has been answered, skipping here avoids the + # redundant skill disk read, reminder re-injection, and duplicate "activate" + # audit. Before that, this call is a retry of the activation call and must + # carry the same reminder, replayed from the record rather than re-read so + # it matches what was audited. run_key is computed once here and threaded # through to the write site in _prepare_model_request. run_key = self._activation_run_key(target) - if self._already_activated(run_context, run_key): - return None + recorded = self._recorded_activation(run_context, run_key) + if recorded is not None: + if self._target_answered(messages, target_index): + return None + return target_index, target, _ActivationResolution(activation=recorded, replayed=True), run_key content = get_original_user_content_text(target.content, target.additional_kwargs) names = target.additional_kwargs.get("skill_references") @@ -495,26 +527,28 @@ Follow this skill before choosing a general workflow. Load supporting resources if activation is None: return None, None - logger.info( - "SkillActivationMiddleware: activating slash skill %s category=%s path=%s hash=%s", - activation.skill_name, - activation.category, - activation.container_file_path, - activation.content_hash, - ) - for item in (activation, *activation.additional_activations): - self._record_activation(request, item, hook=hook) - # Mark this slash message as activated for the run so the tool loop's later - # model calls skip the redundant re-activation (#3861: one activation call, - # many follow-up model calls). A new user slash message keys differently and - # still activates. Overwrite (`=`), not append/accumulate, is intentional: - # _find_activation_target only ever considers the latest real user message as - # an activation target, so there is nothing earlier in the run worth - # remembering once a new activation replaces it — do not "fix" this into a - # set. run_key is the same value already checked in _find_activation_target - # (computed once there, threaded through here) rather than recomputed. - if run_context is not None: - run_context[_SLASH_SKILL_ACTIVATION_RUN_KEY] = run_key + if not resolution.replayed: + logger.info( + "SkillActivationMiddleware: activating slash skill %s category=%s path=%s hash=%s", + activation.skill_name, + activation.category, + activation.container_file_path, + activation.content_hash, + ) + for item in (activation, *activation.additional_activations): + self._record_activation(request, item, hook=hook) + record_skill_usage(getattr(request, "runtime", None), self._usage_snapshot(item)) + # Mark this slash message as activated for the run so the tool loop's later + # model calls skip the redundant re-activation (#3861: one activation call, + # many follow-up model calls). A new user slash message keys differently and + # still activates. Overwrite (`=`), not append/accumulate, is intentional: + # _find_activation_target only ever considers the latest real user message as + # an activation target, so there is nothing earlier in the run worth + # remembering once a new activation replaces it — do not "fix" this into a + # set. run_key is the same value already checked in _find_activation_target + # (computed once there, threaded through here) rather than recomputed. + if run_context is not None: + run_context[_SLASH_SKILL_ACTIVATION_RUN_KEY] = _RecordedActivation(run_key=run_key, activation=activation) activation_msg = self._make_activation_message(target, "\n\n".join(self._build_activation_reminder(item, include_user_request=index == 0) for index, item in enumerate((activation, *activation.additional_activations)))) messages = list(request.messages) messages.insert(target_index, activation_msg) @@ -533,9 +567,6 @@ Follow this skill before choosing a general workflow. Load supporting resources return prepared, None effective = prepared if prepared is not None else request self._resolve_secret_bindings(effective, activation, hook=hook, activation_decisions=activation_decisions, entry_registry=entry_registry) - if activation is not None: - for item in (activation, *activation.additional_activations): - record_skill_usage(getattr(request, "runtime", None), self._usage_snapshot(item)) return effective, activation @staticmethod diff --git a/backend/packages/harness/deerflow/runtime/secret_context.py b/backend/packages/harness/deerflow/runtime/secret_context.py index 93aee30e09..a481f8854d 100644 --- a/backend/packages/harness/deerflow/runtime/secret_context.py +++ b/backend/packages/harness/deerflow/runtime/secret_context.py @@ -179,8 +179,10 @@ _SECRETS_BINDING_AUDIT_KEY = "__skill_secrets_binding_audit" # The reminder is injected into the per-call model request only and never written # back to graph state, so a scan of ``request.messages`` cannot detect a prior # activation on the 2nd..Nth model call — the run context is the only signal that -# survives (mirroring ``_SLASH_SECRET_SOURCE_KEY``). Holds a message id / content -# digest, never a secret value; listed below to keep the redaction guard complete. +# survives (mirroring ``_SLASH_SECRET_SOURCE_KEY``). Holds the message id / content +# digest plus the activation it produced (skill content, never a secret value), so +# a retry of the activation call can replay the same reminder; listed below to +# keep the redaction guard complete. _SLASH_SKILL_ACTIVATION_RUN_KEY = "__slash_skill_activation_run" # Run-context keys whose values are request-scoped secrets and must be stripped diff --git a/backend/tests/test_inline_skill_references.py b/backend/tests/test_inline_skill_references.py index 00dac3a799..b4384c4f14 100644 --- a/backend/tests/test_inline_skill_references.py +++ b/backend/tests/test_inline_skill_references.py @@ -66,10 +66,10 @@ def test_inline_batch_activates_once_with_owner_scoped_secrets_and_usage(catalog async def ahandler(value): return handler(value) - def invoke(): - return asyncio.run(middleware.awrap_model_call(original, ahandler)) if async_call else middleware.wrap_model_call(original, handler) + def invoke(model_request): + return asyncio.run(middleware.awrap_model_call(model_request, ahandler)) if async_call else middleware.wrap_model_call(model_request, handler) - response = invoke() + response = invoke(original) reminder = seen[0].messages[0] assert isinstance(reminder, HumanMessage) assert reminder.additional_kwargs["hide_from_ui"] is True @@ -86,8 +86,10 @@ def test_inline_batch_activates_once_with_owner_scoped_secrets_and_usage(catalog assert [entry["name"] for entry in usages] == names assert journal.record_skill_usage.call_count == count assert context[_SLASH_SKILL_ACTIVATION_RUN_KEY] - invoke() - assert seen[1].messages == original.messages + # The tool loop's next call sees the answered slash message in state. + follow_up = original.override(messages=[*original.messages, response]) + invoke(follow_up) + assert seen[1].messages == follow_up.messages assert journal.record_skill_usage.call_count == count assert context[ACTIVE_SECRETS_CONTEXT_KEY] == {f"KEY_{i}": f"value-{i}" for i in range(count)} diff --git a/backend/tests/test_slash_skills.py b/backend/tests/test_slash_skills.py index 1ce45f0a87..6494ba542b 100644 --- a/backend/tests/test_slash_skills.py +++ b/backend/tests/test_slash_skills.py @@ -4,8 +4,13 @@ from pathlib import Path from types import SimpleNamespace import pytest +from langchain.agents import create_agent from langchain.agents.middleware.types import ModelRequest +from langchain_core.language_models.fake_chat_models import FakeMessagesListChatModel from langchain_core.messages import AIMessage, HumanMessage, ToolMessage +from langchain_core.runnables import Runnable +from langchain_core.tools import tool as as_tool +from pydantic import Field, PrivateAttr from app.channels.commands import KNOWN_CHANNEL_COMMANDS from deerflow.agents.middlewares import skill_activation_middleware as middleware_module @@ -394,6 +399,118 @@ def test_skill_activation_middleware_activates_once_across_tool_loop(monkeypatch assert sum(1 for kwargs in recorded if kwargs.get("action") == "activate") == 1 +class _ProviderUnavailable(Exception): + def __init__(self) -> None: + super().__init__("503 Service Unavailable") + self.status_code = 503 + self.response = SimpleNamespace(status_code=503, headers={}) + + +class _ScriptedChatModel(FakeMessagesListChatModel): + """Fake chat model that records every request and raises scripted errors in order.""" + + failures: list[Exception] = Field(default_factory=list) + _seen_messages: list[list] = PrivateAttr(default_factory=list) + + @property + def seen_messages(self) -> list[list]: + return self._seen_messages + + def bind_tools(self, tools, *, tool_choice=None, **kwargs) -> Runnable: + return self + + def _generate(self, messages, stop=None, run_manager=None, **kwargs): + self._seen_messages.append(list(messages)) + if self.failures: + raise self.failures.pop(0) + return super()._generate(messages, stop=stop, run_manager=run_manager, **kwargs) + + +def _run_retrying_slash_graph(monkeypatch, tmp_path, *, model, asynchronous=False): + from deerflow.agents.middlewares.llm_error_handling_middleware import LLMErrorHandlingMiddleware + from deerflow.config.app_config import AppConfig, LlmCallConfig + from deerflow.config.sandbox_config import SandboxConfig + + skill = _make_skill(tmp_path, "data-analysis", content="# Data Analysis\nUse pandas.") + disk_reads = {"n": 0} + real_read = SkillActivationMiddleware._read_skill_content + + def counting_read(skill_file, skills_root, *, storage=None): + disk_reads["n"] += 1 + return real_read(skill_file, skills_root, storage=storage) + + monkeypatch.setattr(middleware_module, "get_or_new_skill_storage", lambda **kwargs: _make_storage(tmp_path, [skill])) + monkeypatch.setattr(SkillActivationMiddleware, "_read_skill_content", staticmethod(counting_read)) + + @as_tool + def echo(text: str) -> str: + """Echo the text back.""" + return text + + audits = [] + usages = [] + journal = SimpleNamespace(record_middleware=lambda *args, **kwargs: audits.append(kwargs), record_skill_usage=usages.append) + app_config = AppConfig( + sandbox=SandboxConfig(use="test"), + llm_call=LlmCallConfig(retry_max_attempts=3, retry_base_delay_ms=0, retry_cap_delay_ms=0), + ) + # Production order: LLMErrorHandling sits outside SkillActivation, so a retry + # runs SkillActivation's wrap again on the same request. + graph = create_agent( + model=model, + tools=[echo], + middleware=[LLMErrorHandlingMiddleware(app_config=app_config), SkillActivationMiddleware(slash_source_owner_token=_SLASH_SOURCE_OWNER_TOKEN)], + ) + graph_input = {"messages": [HumanMessage(content="/data-analysis analyze uploads/foo.csv", id="msg-1")]} + context = {"__run_journal": journal} + config = {"recursion_limit": 20} + result = asyncio.run(graph.ainvoke(graph_input, context=context, config=config)) if asynchronous else graph.invoke(graph_input, context=context, config=config) + carries_skill = [any(is_slash_skill_activation_reminder(message) and "Use pandas." in message.content for message in messages) for messages in model.seen_messages] + return SimpleNamespace(result=result, carries_skill=carries_skill, disk_reads=disk_reads["n"], audits=audits, usages=usages) + + +@pytest.mark.parametrize("asynchronous", [False, True]) +def test_skill_activation_survives_a_retried_model_call_in_real_agent_graph(monkeypatch, tmp_path, asynchronous): + # The run key is recorded before the model call. When the activation call + # raises, LLMErrorHandling retries by running this wrap again on the same + # request; the retry must still carry SKILL.md, replayed from the recorded + # activation without a second disk read, audit event, or usage record. + model = _ScriptedChatModel( + responses=[ + AIMessage(content="", tool_calls=[{"name": "echo", "args": {"text": "hi"}, "id": "call-1"}]), + AIMessage(content="final answer"), + ], + failures=[_ProviderUnavailable()], + ) + + run = _run_retrying_slash_graph(monkeypatch, tmp_path, model=model, asynchronous=asynchronous) + + # Failed activation call, its retry, then the tool loop's follow-up call. + assert run.carries_skill == [True, True, False] + assert run.result["messages"][-1].content == "final answer" + assert run.disk_reads == 1 + assert sum(1 for kwargs in run.audits if kwargs.get("action") == "activate") == 1 + assert [usage["name"] for usage in run.usages] == ["data-analysis"] + # The failed attempt's stamped response was discarded; the retry's response + # is the one that reaches state and must carry the usage evidence. + assert run.result["messages"][1].additional_kwargs["skill_usage"]["name"] == "data-analysis" + + +def test_skill_activation_survives_an_empty_response_retry_in_real_agent_graph(monkeypatch, tmp_path): + # An empty response is not an exception inside this wrap: the handler returns, + # and LLMErrorHandling raises only after it, outside. The retry must still + # carry SKILL.md even though nothing failed from this middleware's view. + model = _ScriptedChatModel(responses=[AIMessage(content=""), AIMessage(content="final answer")]) + + run = _run_retrying_slash_graph(monkeypatch, tmp_path, model=model) + + assert run.carries_skill == [True, True] + assert run.result["messages"][-1].content == "final answer" + assert run.result["messages"][-1].additional_kwargs["skill_usage"]["name"] == "data-analysis" + assert sum(1 for kwargs in run.audits if kwargs.get("action") == "activate") == 1 + assert len(run.usages) == 1 + + def test_skill_activation_middleware_reactivates_on_new_user_slash_command(monkeypatch, tmp_path): # The run-scoped dedup must not suppress a genuinely new activation: a later # user slash message (distinct id / text) keys differently, so it still @@ -864,9 +981,10 @@ def test_slash_usage_snapshot_is_persisted_on_first_model_response_only(monkeypa middleware = SkillActivationMiddleware(slash_source_owner_token=_SLASH_SOURCE_OWNER_TOKEN) recorded = [] runtime = SimpleNamespace(context={"__run_journal": SimpleNamespace(record_skill_usage=recorded.append)}) - request = _make_model_request([HumanMessage(content="/data-analysis analyze", id="user-1")], runtime=runtime) + user = HumanMessage(content="/data-analysis analyze", id="user-1") - def call(): + def call(messages): + request = _make_model_request(messages, runtime=runtime) response = ModelResponse(result=[AIMessage(content="Analyzing")]) if asynchronous: @@ -876,7 +994,7 @@ def test_slash_usage_snapshot_is_persisted_on_first_model_response_only(monkeypa return asyncio.run(middleware.awrap_model_call(request, handler)) return middleware.wrap_model_call(request, lambda _: response) - first = call() + first = call([user]) snapshot = first.result[0].additional_kwargs["skill_usage"] assert recorded == [snapshot] assert snapshot["activation"] == "slash" @@ -886,4 +1004,5 @@ def test_slash_usage_snapshot_is_persisted_on_first_model_response_only(monkeypa assert snapshot["path"] == "/mnt/skills/custom/data-analysis/SKILL.md" skill.skill_file.write_text("Changed later", encoding="utf-8") assert snapshot["content"] == content - assert "skill_usage" not in call().result[0].additional_kwargs + # The tool loop's next call sees the answered slash message in state. + assert "skill_usage" not in call([user, *first.result]).result[0].additional_kwargs