Merge pull request #2171 into stage-348
Trim session tail response overhead (franksong2702)
This commit is contained in:
commit
39df1a1ef3
5 changed files with 331 additions and 6 deletions
|
|
@ -4,6 +4,8 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- Large session responses now skip the full credential-redaction pass for strings that contain no credential markers, and tail-window session loads no longer send historical `tool_calls` when the returned messages already carry per-message tool metadata. The redaction prefilter and local fallback now mirror the full prefixed-token set handled by the agent redactor, so the faster path keeps the existing hard API redaction boundary while reducing session-switch latency on long transcripts.
|
||||
|
||||
- **PR #2179** (self-built, closes #2177) — NVIDIA NIM no longer 404s when WebUI is configured with `provider: nvidia` and a `nvidia/<model>` id. `resolve_model_provider()` in `api/config.py` had the `_PORTAL_PROVIDERS` guard (Nous, OpenCode-Zen, OpenCode-Go, NVIDIA NIM — providers whose APIs require the full namespaced `provider/model` wire format) sitting **after** the `prefix == config_provider` strip branch. For `model_id="nvidia/nemotron-3-super-120b-a12b"` + `config_provider="nvidia"`, the strip branch fired first and returned the bare `nemotron-...` to NIM, which then 404'd because NIM requires the full path. Same bug class as #854 / #894 (Nous portal). The guard was originally added with NVIDIA in mind but the structural ordering was wrong. Fix is a pure reorder of two `if` blocks — hoist `_PORTAL_PROVIDERS` ahead of the strip — so all portal providers always preserve the full `provider/model` id regardless of whether the prefix happens to equal the provider name. Also closes a latent symmetric bug for the Nous case if a `nous/<model>` id ever entered the catalog. Cross-tool trace against hermes-agent's `hermes_cli/models.py` (line 59, 177, 237-239) and `agent/model_metadata.py:68` confirms the agent CLI sends `nvidia/nemotron-...` verbatim — both tools now agree on the wire format. 118-line regression suite covering the reported case, cross-namespace `qwen/` and `meta/` ids, every static nvidia model in `_PROVIDER_MODELS`, the latent `nous/<model>` ordering pin, and a non-portal-provider regression pin for the anthropic strip behavior. nesquena APPROVED with 200-line end-to-end trace + 12-shape behavioural harness + cross-tool wire-format verification. Reported on Discord by @vishnu in #report-bugs.
|
||||
|
||||
## [v0.51.53] — 2026-05-13 — Release AC (stage-346 — 10-PR contributor batch — stale-stream guard extension + guarded worktree remove + CSP report collector + perf + i18n + ctl fix + defense-in-depth)
|
||||
|
|
@ -41,7 +43,6 @@
|
|||
## [v0.51.52] — 2026-05-12 — Release AB (stage-345 — 2-PR low-risk batch — stream-ownership guard + Refresh-usage button on provider quota card)
|
||||
|
||||
### Fixed
|
||||
|
||||
- **PR #2136** by @LumenYoung — Stale stream writebacks no longer poison the active session transcript. `cancel_stream()` intentionally clears `active_stream_id` early so the UI can accept a follow-up turn while an old worker is unwinding — but the old worker could still return later from `run_conversation()` and persist its stale result over the newer transcript, causing visible transcript / turn journal / `state.db` to disagree (especially around cancel+retry on compressed continuations). Adds a single-line ownership check `_stream_writeback_is_current(session, stream_id)` (token equality against `session.active_stream_id`) and short-circuits both finalize paths: the success path in `_run_agent_streaming` and the cancel-handler path in `cancel_stream()`. When the stream no longer owns the writeback, both paths log `Skipping stale stream/cancel writeback` and return cleanly without persisting. 89-line regression suite in `tests/test_stale_stream_writeback.py`; companion updates to `tests/test_issue1361_cancel_data_loss.py` and `tests/test_sprint42.py` for the new return-without-persist behavior.
|
||||
|
||||
### Added
|
||||
|
|
|
|||
131
api/helpers.py
131
api/helpers.py
|
|
@ -112,7 +112,8 @@ MAX_BODY_BYTES = 20 * 1024 * 1024 # 20MB limit for non-upload POST bodies
|
|||
|
||||
def _build_redact_fn():
|
||||
"""Return a redactor backed by hermes-agent plus local fallback patterns."""
|
||||
# Minimal fallback covering the most common credential prefixes.
|
||||
# Fallback mirrors the agent's known credential prefixes so WebUI API
|
||||
# responses remain a hard redaction boundary even without hermes-agent.
|
||||
# Keep this active even when hermes-agent is importable so API responses do
|
||||
# not regress if the agent redactor misses a token shape.
|
||||
_CRED_RE = _re.compile(
|
||||
|
|
@ -124,10 +125,34 @@ def _build_redact_fn():
|
|||
r"|ghu_[A-Za-z0-9]{10,}" # GitHub user-to-server token
|
||||
r"|ghs_[A-Za-z0-9]{10,}" # GitHub server-to-server token
|
||||
r"|ghr_[A-Za-z0-9]{10,}" # GitHub refresh token
|
||||
r"|xox[baprs]-[A-Za-z0-9-]{10,}" # Slack tokens
|
||||
r"|AIza[A-Za-z0-9_-]{30,}" # Google API keys
|
||||
r"|pplx-[A-Za-z0-9]{10,}" # Perplexity
|
||||
r"|fal_[A-Za-z0-9_-]{10,}" # Fal.ai
|
||||
r"|fc-[A-Za-z0-9]{10,}" # Firecrawl
|
||||
r"|bb_live_[A-Za-z0-9_-]{10,}" # BrowserBase
|
||||
r"|gAAAA[A-Za-z0-9_=-]{20,}" # Codex encrypted tokens
|
||||
r"|AKIA[A-Z0-9]{16}" # AWS Access Key ID
|
||||
r"|xox[baprs]-[A-Za-z0-9-]{10,}" # Slack tokens
|
||||
r"|hf_[A-Za-z0-9]{10,}" # HuggingFace token
|
||||
r"|SG\.[A-Za-z0-9_-]{10,}" # SendGrid API key
|
||||
r"|sk_live_[A-Za-z0-9]{10,}" # Stripe secret key (live)
|
||||
r"|sk_test_[A-Za-z0-9]{10,}" # Stripe secret key (test)
|
||||
r"|rk_live_[A-Za-z0-9]{10,}" # Stripe restricted key
|
||||
r"|SG\.[A-Za-z0-9_-]{10,}" # SendGrid API key
|
||||
r"|hf_[A-Za-z0-9]{10,}" # HuggingFace token
|
||||
r"|r8_[A-Za-z0-9]{10,}" # Replicate API token
|
||||
r"|npm_[A-Za-z0-9]{10,}" # npm access token
|
||||
r"|pypi-[A-Za-z0-9_-]{10,}" # PyPI API token
|
||||
r"|dop_v1_[A-Za-z0-9]{10,}" # DigitalOcean PAT
|
||||
r"|doo_v1_[A-Za-z0-9]{10,}" # DigitalOcean OAuth
|
||||
r"|am_[A-Za-z0-9_-]{10,}" # AgentMail API key
|
||||
r"|sk_[A-Za-z0-9_]{10,}" # ElevenLabs TTS key
|
||||
r"|tvly-[A-Za-z0-9]{10,}" # Tavily search API key
|
||||
r"|exa_[A-Za-z0-9]{10,}" # Exa search API key
|
||||
r"|gsk_[A-Za-z0-9]{10,}" # Groq Cloud API key
|
||||
r"|syt_[A-Za-z0-9]{10,}" # Matrix access token
|
||||
r"|retaindb_[A-Za-z0-9]{10,}" # RetainDB API key
|
||||
r"|hsk-[A-Za-z0-9]{10,}" # Hindsight API key
|
||||
r"|mem0_[A-Za-z0-9]{10,}" # Mem0 Platform API key
|
||||
r"|brv_[A-Za-z0-9]{10,}" # ByteRover API key
|
||||
r")(?![A-Za-z0-9_-])"
|
||||
)
|
||||
_AUTH_HDR_RE = _re.compile(r"(Authorization:\s*Bearer\s+)(\S+)", _re.IGNORECASE)
|
||||
|
|
@ -179,6 +204,102 @@ def _build_redact_fn():
|
|||
_redact_fn_cached = _build_redact_fn()
|
||||
|
||||
|
||||
_SENSITIVE_CASE_MARKERS = (
|
||||
"sk-",
|
||||
"ghp_",
|
||||
"github_pat_",
|
||||
"gho_",
|
||||
"ghu_",
|
||||
"ghs_",
|
||||
"ghr_",
|
||||
"AKIA",
|
||||
"xoxb-",
|
||||
"xoxa-",
|
||||
"xoxp-",
|
||||
"xoxr-",
|
||||
"xoxs-",
|
||||
"AIza",
|
||||
"pplx-",
|
||||
"fal_",
|
||||
"fc-",
|
||||
"bb_live_",
|
||||
"gAAAA",
|
||||
"sk_live_",
|
||||
"sk_test_",
|
||||
"rk_live_",
|
||||
"SG.",
|
||||
"hf_",
|
||||
"r8_",
|
||||
"npm_",
|
||||
"pypi-",
|
||||
"dop_v1_",
|
||||
"doo_v1_",
|
||||
"am_",
|
||||
"sk_",
|
||||
"tvly-",
|
||||
"exa_",
|
||||
"gsk_",
|
||||
"syt_",
|
||||
"retaindb_",
|
||||
"hsk-",
|
||||
"mem0_",
|
||||
"brv_",
|
||||
"eyJ",
|
||||
"-----BEGIN",
|
||||
)
|
||||
_SENSITIVE_LOWER_MARKERS = (
|
||||
"authorization: bearer ",
|
||||
"private key",
|
||||
"postgres://",
|
||||
"postgresql://",
|
||||
"mysql://",
|
||||
"mongodb://",
|
||||
"redis://",
|
||||
"amqp://",
|
||||
"access_token",
|
||||
"refresh_token",
|
||||
"id_token",
|
||||
"api_key",
|
||||
"apikey",
|
||||
"client_secret",
|
||||
"auth_token",
|
||||
"raw_secret",
|
||||
"secret_input",
|
||||
"key_material",
|
||||
"x-amz-signature",
|
||||
"token=",
|
||||
"secret=",
|
||||
"password=",
|
||||
"authorization=",
|
||||
"key=",
|
||||
'"token"',
|
||||
'"secret"',
|
||||
'"password"',
|
||||
'"bearer"',
|
||||
)
|
||||
_SENSITIVE_TELEGRAM_MARKER_RE = _re.compile(r"(?:bot)?\d{8,}:[-A-Za-z0-9_]{30,}")
|
||||
_SENSITIVE_DISCORD_MARKER_RE = _re.compile(r"<@!?\d{17,20}>")
|
||||
_SENSITIVE_PHONE_MARKER_RE = _re.compile(r"(?<![A-Za-z0-9])\+[1-9]\d{6,14}(?![A-Za-z0-9])")
|
||||
|
||||
|
||||
def _might_contain_sensitive_text(text: str) -> bool:
|
||||
"""Cheap prefilter before the full agent+fallback redaction pass."""
|
||||
if not isinstance(text, str) or not text:
|
||||
return False
|
||||
if any(marker in text for marker in _SENSITIVE_CASE_MARKERS):
|
||||
return True
|
||||
lower = text.lower()
|
||||
if any(marker in lower for marker in _SENSITIVE_LOWER_MARKERS):
|
||||
return True
|
||||
if ":" in text and _SENSITIVE_TELEGRAM_MARKER_RE.search(text):
|
||||
return True
|
||||
if "<@" in text and _SENSITIVE_DISCORD_MARKER_RE.search(text):
|
||||
return True
|
||||
if "+" in text and _SENSITIVE_PHONE_MARKER_RE.search(text):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _redact_text(text: str, *, _enabled: bool | None = None) -> str:
|
||||
"""Redact sensitive text from API responses. Respects api_redact_enabled setting.
|
||||
|
||||
|
|
@ -194,6 +315,8 @@ def _redact_text(text: str, *, _enabled: bool | None = None) -> str:
|
|||
_enabled = bool(load_settings().get("api_redact_enabled", True))
|
||||
if not _enabled:
|
||||
return text
|
||||
if not _might_contain_sensitive_text(text):
|
||||
return text
|
||||
return _redact_fn_cached(text)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1666,6 +1666,24 @@ def _is_messaging_session_record(session) -> bool:
|
|||
return _is_known_messaging_source(raw)
|
||||
|
||||
|
||||
def _messages_include_tool_metadata(messages) -> bool:
|
||||
"""Return true when returned messages can reconstruct their own tool cards."""
|
||||
if not isinstance(messages, list):
|
||||
return False
|
||||
for msg in messages:
|
||||
if not isinstance(msg, dict) or msg.get("role") != "assistant":
|
||||
continue
|
||||
if isinstance(msg.get("tool_calls"), list) and msg.get("tool_calls"):
|
||||
return True
|
||||
content = msg.get("content")
|
||||
if isinstance(content, list) and any(
|
||||
isinstance(part, dict) and part.get("type") == "tool_use"
|
||||
for part in content
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _is_messaging_session_id(sid: str) -> bool:
|
||||
"""Detect messaging-backed sessions from WebUI metadata or Agent rows."""
|
||||
try:
|
||||
|
|
@ -3363,9 +3381,19 @@ def handle_get(handler, parsed) -> bool:
|
|||
_persisted_cl = _fb_cl
|
||||
except Exception:
|
||||
pass
|
||||
_session_tool_calls = getattr(s, "tool_calls", []) if load_messages else []
|
||||
if (
|
||||
load_messages
|
||||
and msg_limit is not None
|
||||
and _messages_include_tool_metadata(_truncated_msgs)
|
||||
):
|
||||
# The browser ignores session-level tool_calls when the returned
|
||||
# messages already carry per-message tool metadata. Avoid sending
|
||||
# the full historical list with a small tail window.
|
||||
_session_tool_calls = []
|
||||
raw = s.compact() | {
|
||||
"messages": _truncated_msgs,
|
||||
"tool_calls": getattr(s, "tool_calls", []) if load_messages else [],
|
||||
"tool_calls": _session_tool_calls,
|
||||
"active_stream_id": getattr(s, "active_stream_id", None),
|
||||
"pending_user_message": getattr(s, "pending_user_message", None),
|
||||
"pending_attachments": getattr(s, "pending_attachments", []) if load_messages else [],
|
||||
|
|
|
|||
|
|
@ -107,6 +107,87 @@ def test_redact_value_list():
|
|||
assert result[1]["content"] == "safe text"
|
||||
|
||||
|
||||
def test_redact_text_skips_full_redactor_for_safe_text(monkeypatch):
|
||||
"""Large ordinary transcript text should not pay the full redactor pass."""
|
||||
import api.helpers as helpers
|
||||
|
||||
calls = []
|
||||
monkeypatch.setattr(helpers, "_redact_fn_cached", lambda text: calls.append(text) or text)
|
||||
|
||||
safe_text = "ordinary session transcript without credential markers\n" * 500
|
||||
assert helpers._redact_text(safe_text, _enabled=True) == safe_text
|
||||
assert calls == []
|
||||
|
||||
|
||||
def test_redact_text_still_runs_full_redactor_for_sensitive_markers(monkeypatch):
|
||||
"""The cheap prefilter must preserve the hard redaction boundary."""
|
||||
import api.helpers as helpers
|
||||
|
||||
calls = []
|
||||
|
||||
def fake_redactor(text):
|
||||
calls.append(text)
|
||||
return text.replace(_FAKE_SK_KEY, "sk-Tes...cdef")
|
||||
|
||||
monkeypatch.setattr(helpers, "_redact_fn_cached", fake_redactor)
|
||||
|
||||
result = helpers._redact_text(f"token={_FAKE_SK_KEY}", _enabled=True)
|
||||
|
||||
assert _FAKE_SK_KEY not in result
|
||||
assert calls == [f"token={_FAKE_SK_KEY}"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("prefix,suffix", [
|
||||
("sk-", "TestCredential1234567890"),
|
||||
("ghp_", "TestCredential1234567890"),
|
||||
("github_pat_", "TestCredential_1234567890"),
|
||||
("gho_", "TestCredential1234567890"),
|
||||
("ghu_", "TestCredential1234567890"),
|
||||
("ghs_", "TestCredential1234567890"),
|
||||
("ghr_", "TestCredential1234567890"),
|
||||
("xoxb-", "TestCredential1234567890"),
|
||||
("xoxa-", "TestCredential1234567890"),
|
||||
("xoxp-", "TestCredential1234567890"),
|
||||
("xoxr-", "TestCredential1234567890"),
|
||||
("xoxs-", "TestCredential1234567890"),
|
||||
("AIza", "TestCredential1234567890abcdefghi"),
|
||||
("pplx-", "TestCredential1234567890"),
|
||||
("fal_", "TestCredential1234567890"),
|
||||
("fc-", "TestCredential1234567890"),
|
||||
("bb_live_", "TestCredential1234567890"),
|
||||
("gAAAA", "TestCredential1234567890abcd"),
|
||||
("AKIA", "TESTCREDENTIAL12"),
|
||||
("sk_" + "live_", "TestCredential1234567890"),
|
||||
("sk_" + "test_", "TestCredential1234567890"),
|
||||
("rk_" + "live_", "TestCredential1234567890"),
|
||||
("SG.", "TestCredential1234567890"),
|
||||
("hf_", "TestCredential1234567890"),
|
||||
("r8_", "TestCredential1234567890"),
|
||||
("npm_", "TestCredential1234567890"),
|
||||
("pypi-", "TestCredential1234567890"),
|
||||
("dop_v1_", "TestCredential1234567890"),
|
||||
("doo_v1_", "TestCredential1234567890"),
|
||||
("am_", "TestCredential1234567890"),
|
||||
("sk_", "TestCredential1234567890"),
|
||||
("tvly-", "TestCredential1234567890"),
|
||||
("exa_", "TestCredential1234567890"),
|
||||
("gsk_", "TestCredential1234567890"),
|
||||
("syt_", "TestCredential1234567890"),
|
||||
("retaindb_", "TestCredential1234567890"),
|
||||
("hsk-", "TestCredential1234567890"),
|
||||
("mem0_", "TestCredential1234567890"),
|
||||
("brv_", "TestCredential1234567890"),
|
||||
])
|
||||
def test_redact_text_prefilter_covers_known_prefixed_credentials(prefix, suffix):
|
||||
"""Every known prefix must still reach the hard redactor."""
|
||||
import api.helpers as helpers
|
||||
|
||||
token = prefix + suffix
|
||||
result = helpers._redact_text(f"credential={token}", _enabled=True)
|
||||
|
||||
assert token not in result
|
||||
|
||||
|
||||
def test_redact_value_works_with_legacy_agent_redact_signature(monkeypatch):
|
||||
"""_redact_text must tolerate older redact_sensitive_text(text) signatures."""
|
||||
fake_agent = types.ModuleType("agent")
|
||||
|
|
|
|||
92
tests/test_session_tail_payload.py
Normal file
92
tests/test_session_tail_payload.py
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
from urllib.parse import urlparse
|
||||
|
||||
|
||||
class _FakeSession:
|
||||
def __init__(self, messages):
|
||||
self.session_id = "tail_payload_001"
|
||||
self.title = "Tail payload"
|
||||
self.workspace = "/tmp"
|
||||
self.model = "gpt-test"
|
||||
self.model_provider = None
|
||||
self.messages = messages
|
||||
self.tool_calls = [
|
||||
{"name": "old-tool", "snippet": "historical snippet", "assistant_msg_idx": 0}
|
||||
]
|
||||
self.input_tokens = 0
|
||||
self.output_tokens = 0
|
||||
self.estimated_cost = 0
|
||||
self.context_length = 1
|
||||
self.threshold_tokens = 0
|
||||
self.last_prompt_tokens = 0
|
||||
self.active_stream_id = None
|
||||
self.pending_user_message = None
|
||||
self.pending_attachments = []
|
||||
self.pending_started_at = None
|
||||
self.composer_draft = {}
|
||||
|
||||
def compact(self):
|
||||
return {
|
||||
"session_id": self.session_id,
|
||||
"title": self.title,
|
||||
"workspace": self.workspace,
|
||||
"model": self.model,
|
||||
"model_provider": self.model_provider,
|
||||
"message_count": len(self.messages),
|
||||
"context_length": self.context_length,
|
||||
"threshold_tokens": self.threshold_tokens,
|
||||
"last_prompt_tokens": self.last_prompt_tokens,
|
||||
"active_stream_id": self.active_stream_id,
|
||||
"pending_user_message": self.pending_user_message,
|
||||
"composer_draft": self.composer_draft,
|
||||
}
|
||||
|
||||
|
||||
def _invoke(session):
|
||||
import api.routes as routes
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_j(_handler, data, status=200, extra_headers=None):
|
||||
captured["data"] = data
|
||||
captured["status"] = status
|
||||
return data
|
||||
|
||||
parsed = urlparse("/api/session?session_id=tail_payload_001&messages=1&resolve_model=0&msg_limit=1")
|
||||
with patch("api.routes.get_session", return_value=session), \
|
||||
patch("api.routes._clear_stale_stream_state", return_value=False), \
|
||||
patch("api.routes._lookup_cli_session_metadata", return_value={}), \
|
||||
patch("api.routes.redact_session_data", side_effect=lambda raw: raw), \
|
||||
patch("api.routes.j", side_effect=fake_j):
|
||||
routes.handle_get(SimpleNamespace(), parsed)
|
||||
return captured["data"]["session"]
|
||||
|
||||
|
||||
def test_tail_window_omits_historical_tool_calls_when_messages_have_tool_metadata():
|
||||
session = _FakeSession([
|
||||
{"role": "user", "content": "older"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "visible",
|
||||
"tool_calls": [{"id": "call_1", "function": {"name": "tool", "arguments": "{}"}}],
|
||||
},
|
||||
])
|
||||
|
||||
payload = _invoke(session)
|
||||
|
||||
assert payload["messages"] == [session.messages[-1]]
|
||||
assert payload["tool_calls"] == []
|
||||
assert payload["_messages_truncated"] is True
|
||||
|
||||
|
||||
def test_tail_window_keeps_session_tool_calls_for_legacy_messages_without_metadata():
|
||||
session = _FakeSession([
|
||||
{"role": "user", "content": "older"},
|
||||
{"role": "assistant", "content": "visible legacy message"},
|
||||
])
|
||||
|
||||
payload = _invoke(session)
|
||||
|
||||
assert payload["messages"] == [session.messages[-1]]
|
||||
assert payload["tool_calls"] == session.tool_calls
|
||||
Loading…
Reference in a new issue