fix(skills): don't attach model tracing to the in-graph skill security scan (#4252)

* fix(skills): don't attach model tracing to the in-graph skill security scan

* fix(skills): pass attach_tracing explicitly from the in-graph scan call site

Follow the tracing INVARIANT's own convention rather than detecting the call
context: scan_skill_content takes an attach_tracing flag, and _scan_or_raise --
the single in-graph choke point -- passes False. Standalone callers (Gateway
skill routes, installer) keep the default True.

The INVARIANT list named four sites and asks that new in-graph calls be added
to it; record this fifth one so a future audit of that list finds it.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

backend/packages/harness/deerflow/agents/lead_agent/agent.py

@@ -13,8 +13,12 @@ middleware reachable from this graph (e.g. ``TitleMiddleware``) — MUST pass
 Forgetting that flag emits duplicate spans (one rooted at the graph, one at
 the model) AND prevents the Langfuse handler's ``propagate_attributes``
 path from firing, so ``session_id`` / ``user_id`` never reach the trace.
-The four current sites are: bootstrap agent, default agent, summarization
+The five current sites are: bootstrap agent, default agent, summarization
-middleware, and the async path inside ``TitleMiddleware``. Any new in-graph
+middleware, the async path inside ``TitleMiddleware``, and the skill security
+scanner reached from the ``skill_manage`` tool (``skills/security_scanner.py``'s
+``scan_skill_content``, which is dual-use: ``_scan_or_raise`` in
+``tools/skill_manage_tool.py`` is the in-graph choke point and passes the flag,
+while its standalone callers keep the default). Any new in-graph
 ``create_chat_model`` call must add to this list and pass the flag.
 """
 

backend/packages/harness/deerflow/skills/security_scanner.py

@@ -96,8 +96,19 @@ async def scan_skill_content(
     location: str = SKILL_MD_FILE,
     app_config: AppConfig | None = None,
     static_findings: list[dict[str, Any]] | None = None,
+    attach_tracing: bool = True,
 ) -> ScanResult:
-    """Screen skill content before it is written to disk."""
+    """Screen skill content before it is written to disk.
+
+    ``attach_tracing`` follows the tracing INVARIANT in
+    ``agents/lead_agent/agent.py``: in-graph callers must pass ``False`` because
+    the graph root already attached the callbacks, and attaching again at the
+    model emits duplicate spans *and* blocks the Langfuse handler's
+    ``propagate_attributes`` path. This function is dual-use, so the flag is the
+    caller's to set — the in-graph choke point is ``_scan_or_raise`` in
+    ``tools/skill_manage_tool.py``. Standalone callers (Gateway skill routes,
+    ``skills/installer.py``) have no root to inherit from and keep the default.
+    """
     rubric = (
         "You are a security reviewer for AI agent skills. "
         "Classify the content as allow, warn, or block. "
@@ -112,7 +123,8 @@ async def scan_skill_content(
     try:
         config = app_config or get_app_config()
         model_name = config.skill_evolution.moderation_model_name
-        model = create_chat_model(name=model_name, thinking_enabled=False, app_config=config) if model_name else create_chat_model(thinking_enabled=False, app_config=config)
+        model_kwargs = {"thinking_enabled": False, "app_config": config, "attach_tracing": attach_tracing}
+        model = create_chat_model(name=model_name, **model_kwargs) if model_name else create_chat_model(**model_kwargs)
         response = await model.ainvoke(
             [
                 {"role": "system", "content": rubric},

backend/packages/harness/deerflow/tools/skill_manage_tool.py

@@ -64,7 +64,9 @@ def _history_record(*, action: str, file_path: str, prev_content: str | None, ne
 
 
 async def _scan_or_raise(content: str, *, executable: bool, location: str, static_findings: list[StaticFinding] | None = None) -> dict[str, Any]:
-    result = await scan_skill_content(content, executable=executable, location=location, static_findings=static_findings or [])
+    # In-graph: the graph root already attached tracing (see the INVARIANT in
+    # agents/lead_agent/agent.py), so the scan model must not attach it again.
+    result = await scan_skill_content(content, executable=executable, location=location, static_findings=static_findings or [], attach_tracing=False)
     if result.decision == "block":
         raise ValueError(f"Security scan blocked the write: {result.reason}")
     if executable and result.decision != "allow":

backend/tests/test_security_scanner.py

@@ -17,8 +17,13 @@ def _make_env(monkeypatch, response_content):
             return fake_response
 
     model = FakeModel()
+
+    def _fake_create_chat_model(**kwargs):
+        model.create_kwargs = kwargs
+        return model
+
     monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config)
-    monkeypatch.setattr("deerflow.skills.security_scanner.create_chat_model", lambda **kwargs: model)
+    monkeypatch.setattr("deerflow.skills.security_scanner.create_chat_model", _fake_create_chat_model)
     return model
 
 
@@ -142,6 +147,39 @@ async def test_scan_distinguishes_unparseable_executable(monkeypatch):
     assert "unparseable" in result.reason
 
 
+# --- tracing wiring: in-graph vs standalone (see the INVARIANT in
+# packages/harness/deerflow/agents/lead_agent/agent.py and the Tracing System
+# section of backend/AGENTS.md) ---
+
+
+@pytest.mark.anyio
+async def test_scan_skill_content_forwards_attach_tracing_to_the_model(monkeypatch):
+    """In-graph callers pass ``attach_tracing=False``; it must reach the factory.
+
+    The graph root already attached the callbacks, so attaching again at the model
+    emits duplicate spans and blocks the Langfuse handler's ``propagate_attributes``
+    path, meaning session_id/user_id never land on the trace.
+    """
+    model = _make_env(monkeypatch, '{"decision":"allow","reason":"ok"}')
+    result = await scan_skill_content(SKILL_CONTENT, executable=False, attach_tracing=False)
+    assert result.decision == "allow"
+    assert model.create_kwargs["attach_tracing"] is False
+
+
+@pytest.mark.anyio
+async def test_scan_skill_content_attaches_model_tracing_by_default(monkeypatch):
+    """Standalone callers (Gateway skill routes, installer) have no graph root to
+    inherit from, so the default keeps model-level attachment.
+
+    Anchors the other direction of the change: narrowing the fix into an
+    unconditional ``attach_tracing=False`` would silently drop their spans.
+    """
+    model = _make_env(monkeypatch, '{"decision":"allow","reason":"ok"}')
+    result = await scan_skill_content(SKILL_CONTENT, executable=False)
+    assert result.decision == "allow"
+    assert model.create_kwargs["attach_tracing"] is True
+
+
 def _make_unavailable_env(monkeypatch, *, security_fail_closed):
     config = SimpleNamespace(
         skill_evolution=SimpleNamespace(

backend/tests/test_skill_manage_tool.py

@@ -340,3 +340,43 @@ def test_skill_manage_per_user_isolation(monkeypatch, tmp_path):
     # No cross-contamination
     assert not (tmp_path / "users" / "alice" / "skills" / "custom" / "bob-skill").exists()
     assert not (tmp_path / "users" / "bob" / "skills" / "custom" / "alice-skill").exists()
+
+
+# --- tracing wiring: the in-graph choke point (see the INVARIANT in
+# packages/harness/deerflow/agents/lead_agent/agent.py) ---
+
+
+def test_scan_or_raise_does_not_attach_model_tracing(monkeypatch, tmp_path):
+    """``_scan_or_raise`` is the in-graph choke point for the skill security scan.
+
+    The graph root already attached the tracing callbacks, so the scan model must
+    not attach them again: double-attaching emits duplicate spans and blocks the
+    Langfuse handler's ``propagate_attributes`` path, so session_id/user_id never
+    reach the trace. Drives the real ``scan_skill_content`` rather than stubbing it,
+    so the flag is pinned all the way to the model factory.
+    """
+    config = _make_config(tmp_path / "skills")
+    monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config)
+
+    create_kwargs = {}
+
+    class FakeModel:
+        async def ainvoke(self, *args, **kwargs):
+            return SimpleNamespace(content='{"decision":"allow","reason":"ok"}')
+
+    def _fake_create_chat_model(**kwargs):
+        create_kwargs.update(kwargs)
+        return FakeModel()
+
+    monkeypatch.setattr("deerflow.skills.security_scanner.create_chat_model", _fake_create_chat_model)
+
+    result = anyio.run(
+        lambda: skill_manage_module._scan_or_raise(
+            _skill_content("demo-skill"),
+            executable=False,
+            location="demo-skill/SKILL.md",
+        )
+    )
+
+    assert result["decision"] == "allow"
+    assert create_kwargs["attach_tracing"] is False