Daoyuan Li 2fa0505070
fix(skills): activate a slash skill once per run, not per model call (#4103)
* fix(skills): activate a slash skill once per run, not per model call

SkillActivationMiddleware injects the activation reminder for a slash
command via request.override(messages=...), which LangChain's create_agent
uses for a single model call and never writes back to graph state. The
dedup guard scans request.messages for a prior reminder, but model_node
rebuilds request.messages fresh from persisted state on every tool-loop
step, so the reminder is never present on the 2nd..Nth model call of a
turn. Every model call therefore re-parsed the command, re-read SKILL.md
from disk, re-injected the multi-KB body, and re-recorded an "activate"
audit event, despite the code intending a single activation per run
(#3861 semantics: one activation call, many follow-up model calls).

Key the dedup off the run context instead, which LangGraph threads
through every model-node call of a run (the same durable signal the
request-scoped secret source already uses). The activation call records
the slash message's identity in context; later calls for the same message
skip re-activation. A new user slash message keys differently and still
activates. Secret binding is unaffected: it already re-resolves from the
persisted slash source on every call.

Adds regression tests that rebuild the real multi-call turn state and
assert a single activation across the tool loop, plus a test proving a
new slash command still activates.

* fix(skills): address review nits on run-scoped activation dedup

- Extract _already_activated(run_context, run_key) so the dedup check
  mirrors the existing _has_existing_activation_for_target sibling
  instead of an inline dense conditional.
- Compute _activation_run_key() once in _find_activation_target and
  thread it through _prepare_model_request instead of recomputing it
  at the write site, making the "same key for check and write"
  invariant explicit in the code rather than implicit.
- Document why the run-context write is an overwrite rather than an
  append/set: only the latest real user message is ever considered an
  activation target, so there is nothing earlier in the run worth
  preserving.
- Add a regression test locking in the degraded-path contract: when
  runtime.context is None, the middleware still activates per-call
  instead of crashing or wrongly no-op'ing.
2026-07-13 18:38:32 +08:00

115 lines
5.0 KiB
Python

"""Request-scoped secret carrier in the run context (issue #3861).
Callers pass per-request secrets out-of-band in ``config.context.secrets`` — a
mapping of name -> value. The value never enters the prompt, tool arguments, or
the executed command string; it is injected as an environment variable into a
skill's sandbox subprocess only when an activated skill declares it via the
``required-secrets`` frontmatter field.
This module centralises the reserved key name and safe extraction so the carrier
contract lives in one place, consumed by the skill-activation middleware (to
build the per-turn injection set) and the tracing redactor (to strip it from
trace payloads).
"""
from __future__ import annotations
from typing import Any
# Reserved sub-key of the run context that holds request-scoped secrets supplied
# by the caller. Source of truth for what a skill *may* receive.
SECRETS_CONTEXT_KEY = "secrets"
# Reserved sub-key holding the secrets resolved for the currently activated skill
# (binding point A). Written by the skill-activation middleware, read by the bash
# tool. Both reserved keys are stripped from trace payloads (see tracing redactor).
ACTIVE_SECRETS_CONTEXT_KEY = "__active_skill_secrets"
def _string_pairs(raw: Any) -> dict[str, str]:
if not isinstance(raw, dict):
return {}
return {key: value for key, value in raw.items() if isinstance(key, str) and isinstance(value, str)}
def extract_request_secrets(context: Any) -> dict[str, str]:
"""Return the caller-supplied request-scoped secrets mapping, or ``{}``.
Only string-keyed, string-valued entries are kept; anything else is ignored
so a malformed carrier can never crash secret resolution or injection.
"""
if not isinstance(context, dict):
return {}
return _string_pairs(context.get(SECRETS_CONTEXT_KEY))
def read_active_secrets(context: Any) -> dict[str, str]:
"""Return the secrets resolved for the active skill (the per-run injection
set), or ``{}``. Read by the bash tool to build the subprocess env."""
if not isinstance(context, dict):
return {}
return _string_pairs(context.get(ACTIVE_SECRETS_CONTEXT_KEY))
# Private run-context keys the skill-activation middleware uses to carry secret
# bindings across a run. Only ``secrets`` / ``__active_skill_secrets`` hold
# values; the binding-source and audit keys hold names only. All are listed so
# the redaction allowlist stays a complete guard even if a future edit starts
# storing a value under one of the name-only keys.
_SLASH_SECRET_SOURCE_KEY = "__slash_skill_secret_source"
_SECRETS_BINDING_AUDIT_KEY = "__skill_secrets_binding_audit"
# Identity of the latest slash activation that has already fired in this run, so
# the reminder injection, skill disk read, and ``activate`` audit event happen
# once per user slash command rather than on every model call of the tool loop.
# 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.
_SLASH_SKILL_ACTIVATION_RUN_KEY = "__slash_skill_activation_run"
# Run-context keys whose values are request-scoped secrets and must be stripped
# before a context mapping is serialized anywhere observable (traces, logs).
REDACTED_CONTEXT_KEYS = frozenset(
{
SECRETS_CONTEXT_KEY,
ACTIVE_SECRETS_CONTEXT_KEY,
_SLASH_SECRET_SOURCE_KEY,
_SECRETS_BINDING_AUDIT_KEY,
_SLASH_SKILL_ACTIVATION_RUN_KEY,
}
)
def redact_secret_context_keys(context: Any) -> Any:
"""Return a shallow copy of ``context`` with secret-bearing keys removed.
Defensive helper for any code path that serializes the run context into an
observable surface. DeerFlow's own trace-metadata builder never copies the
context, so this is belt-and-suspenders for future call sites and custom
tracer configurations.
"""
if not isinstance(context, dict):
return context
return {key: value for key, value in context.items() if key not in REDACTED_CONTEXT_KEYS}
def redact_config_secrets(config: Any) -> Any:
"""Return a copy of a run config safe to persist or echo back to clients.
The request config (``body.config``) is stored verbatim on the run record
(``runs.kwargs_json``) and echoed by the run API. Strip the secret-bearing
keys from its ``context`` so a request-scoped secret is never persisted or
returned, while the live config that drives the run (built separately) keeps
them. Non-dict / context-less configs pass through unchanged.
"""
if not isinstance(config, dict):
return config
context = config.get("context")
if not isinstance(context, dict):
return config
redacted = dict(config)
redacted["context"] = redact_secret_context_keys(context)
return redacted