mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 19:20:16 +00:00
Merge pull request #1777 from nesquena/stage-309
v0.51.15 — 4-PR batch (#1762, #1767, #1769, #1770)
This commit is contained in:
@@ -1,5 +1,33 @@
|
||||
# Hermes Web UI -- Changelog
|
||||
|
||||
## [v0.51.15] — 2026-05-07 — 4-PR contributor batch + 1 self-built (cron spawn migration, context menu, codex quota, model prefix)
|
||||
|
||||
### Fixed
|
||||
|
||||
- **PR #1767** by @Michaelyklam — Use `spawn` for manual cron subprocesses (closes #1754, the architectural follow-up filed in v0.51.12). One-line context change `multiprocessing.get_context("fork")` → `"spawn"` at `api/routes.py:367` plus +207 LOC of regression coverage in `tests/test_issue1574_cron_profile_lock.py`. Validates: (a) source-level pin that the helper uses spawn, (b) end-to-end harness showing `fork` deadlocks on a parent-thread-held lock while `spawn` succeeds, (c) drain-large-result regression preserved, (d) executes-under-selected-profile-home regression preserved. **Auto-fix applied at stage**: 2 of the 5 tests fail on dev machines with an editable `hermes_agent` install (the spawn child resolves the real `cron.scheduler` first instead of the fake one written under `HERMES_WEBUI_AGENT_DIR`). Added `_real_hermes_agent_editable_install_present()` detector using `importlib.util.find_spec` origin check + `pytest.skip` guard. Tests skip on dev (where they cannot work as designed) and run cleanly on CI (where no editable install exists). Closes the fork-from-multi-threaded-WebUI hazard class noted in #1754: import-lock and logging-lock inheritance no longer apply, since spawn starts a fresh interpreter.
|
||||
- **PR #1770** by @Michaelyklam — Surface Codex usage exhaustion errors (closes #1765). New `quota_exhausted` SSE event for Codex 429/quota responses replaces the previous behavior (empty turn with no inline error) with a clear inline error card. `_classify_provider_error()` distinguishes quota-exhaustion (requires re-auth) from transient rate-limit (just needs to wait) — Opus stage-309 verified the classifier order (quota check first, rate-limit is `not _is_quota AND ...`) preserves the distinction. Detection covers Codex OAuth shapes: "plan limit reached", "usage_limit_exceeded", "reached the limit of messages", "used up your usage", plus the multi-token fallback. Both error paths properly clean up runtime state (INFLIGHT, approval/clarify pollers via `finally` block) and run `_materialize_pending_user_turn_before_error()` before `pending_user_message = None` clearing — preserving the user-turn data-loss fix from PR #1760 (v0.51.14). 62 LOC test coverage in `tests/test_issue1765_codex_quota.py`. Includes 2 PNG screenshots.
|
||||
- **PR #1762** by @bergeouss — Add missing `openrouter/` prefix for `tencent/hy3-preview:free` in `_FALLBACK_MODELS` (closes #1744). Pure data fix; resolves the model to the right provider. Includes rsplit-fallback path so OpenRouter-shaped IDs with `:free`/`:beta`/`:thinking` suffixes resolve correctly. **One edge case filed as follow-up #1776** (Opus stage-309 noted: `@custom:<key>:<model>:free` mis-resolves because the rsplit-fallback skips on `custom:` provider hint — uncommon combination, non-blocking).
|
||||
|
||||
### Added
|
||||
|
||||
- **PR #1769** by @nesquena-hermes — Three high-leverage context-menu essentials from #1764 (self-built, **independently APPROVED by @nesquena** at exact head SHA `102157bc`). Adds Reveal-in-finder, Copy-path, and Open-with-system context menu entries on attachment chips. Two new endpoints `_handle_file_reveal` + `_handle_file_path` in `api/routes.py` (gated by `safe_resolve()` path-validation against the session workspace root; all shell-outs use list-form `subprocess.Popen([...])` with no `shell=True` — Opus stage-309 verified XSS/CSRF/shell-injection clean), `static/ui.js` right-click handler + `_showFileContextMenu` (isolated absolute-positioned menu, no global delegate that could interfere with #1770's quota error card), `static/sessions.js` integration, locale strings × 6 in `static/i18n.js`. 343 LOC test coverage in `tests/test_1764_context_menu_essentials.py`.
|
||||
|
||||
### Tests
|
||||
|
||||
4662 → **4694 collected** (+32 across 4 new test files plus regression coverage tightening). 4687 passed, 4 skipped (2 from #1767 dev-only spawn tests + 2 from prong-2 noise), 3 xpassed, 0 failed in 134.82s.
|
||||
|
||||
### Pre-release verification
|
||||
|
||||
- All 4 PRs CI-green individually.
|
||||
- Auto-fix on #1767 verified (3 passed, 2 skipped on dev — would be 5 passed on CI).
|
||||
- `node -c` clean on all 4 changed JS files (`static/ui.js`, `static/messages.js`, `static/i18n.js`, `static/sessions.js`).
|
||||
- pytest: 4687 passed, 0 failed (single clean run, ~135s).
|
||||
- `scripts/run-browser-tests.sh`: all 11 endpoints PASS on isolated port 8789.
|
||||
- Pre-stamp re-fetch: all 4 PR heads still match local rebases — no late commits.
|
||||
- Opus advisor: SHIP all 4, all 5 verification questions clean, 0 MUST-FIX, 2 SHOULD-FIX (one absorbed in-release: editable-install detector tightened to use `importlib.util.find_spec`-origin check; one filed as follow-up #1776).
|
||||
|
||||
Closes #1744, #1754, #1764, #1765.
|
||||
|
||||
## [v0.51.14] — 2026-05-06 — 4-PR contributor batch
|
||||
|
||||
### Fixed
|
||||
|
||||
+1
-1
@@ -2,7 +2,7 @@
|
||||
|
||||
> Web companion to the Hermes Agent CLI. Same workflows, browser-native.
|
||||
>
|
||||
> Last updated: v0.51.14 (May 6, 2026) — 4662 tests collected — 4-PR contributor batch (#1756, #1757, #1760, #1761)
|
||||
> Last updated: v0.51.15 (May 7, 2026) — 4694 tests collected — 4-PR contributor batch + 1 self-built (#1762, #1767, #1769, #1770)
|
||||
> Test source: `pytest tests/ --collect-only -q`
|
||||
> Per-version detail: see [CHANGELOG.md](./CHANGELOG.md)
|
||||
|
||||
|
||||
+2
-2
@@ -1835,8 +1835,8 @@ Bridged CLI sessions:
|
||||
|
||||
---
|
||||
|
||||
*Last updated: v0.51.14, May 6, 2026*
|
||||
*Total automated tests collected: 4662*
|
||||
*Last updated: v0.51.15, May 7, 2026*
|
||||
*Total automated tests collected: 4694*
|
||||
*Regression gate: tests/test_regressions.py*
|
||||
*Run: pytest tests/ -v --timeout=60*
|
||||
*Source: <repo>/*
|
||||
|
||||
+11
-1
@@ -1368,8 +1368,18 @@ def resolve_model_provider(model_id: str) -> tuple:
|
||||
# resolve credentials in streaming.py).
|
||||
# Use rsplit to handle provider_ids that contain ':' (e.g. custom:my-key).
|
||||
# With rsplit, "@custom:my-key:model" → provider="custom:my-key", model="model".
|
||||
# BUT: model IDs that end in :free / :beta / :thinking collide with the
|
||||
# rsplit grammar (e.g. "@openrouter:tencent/hy3-preview:free" would split
|
||||
# into provider="openrouter:tencent/hy3-preview", model="free"). Guard
|
||||
# against that by falling back to split(":") when the rsplit result is not
|
||||
# a recognised provider (#1744).
|
||||
if model_id.startswith("@") and ":" in model_id:
|
||||
provider_hint, bare_model = model_id[1:].rsplit(":", 1)
|
||||
inner = model_id[1:]
|
||||
provider_hint, bare_model = inner.rsplit(":", 1)
|
||||
if (provider_hint not in _PROVIDER_MODELS
|
||||
and provider_hint not in _PROVIDER_DISPLAY
|
||||
and not provider_hint.startswith("custom:")):
|
||||
provider_hint, bare_model = inner.split(":", 1)
|
||||
return bare_model, provider_hint, None
|
||||
|
||||
if "/" in model_id:
|
||||
|
||||
+40
-2
@@ -364,7 +364,7 @@ def _run_cron_job_in_profile_subprocess(job, execution_profile_home):
|
||||
import multiprocessing
|
||||
import queue
|
||||
|
||||
ctx = multiprocessing.get_context("fork")
|
||||
ctx = multiprocessing.get_context("spawn")
|
||||
result_queue = ctx.Queue(maxsize=1)
|
||||
process = ctx.Process(
|
||||
target=_cron_job_subprocess_main,
|
||||
@@ -4022,6 +4022,9 @@ def handle_post(handler, parsed) -> bool:
|
||||
if parsed.path == "/api/file/reveal":
|
||||
return _handle_file_reveal(handler, body)
|
||||
|
||||
if parsed.path == "/api/file/path":
|
||||
return _handle_file_path(handler, body)
|
||||
|
||||
# ── Workspace management (POST) ──
|
||||
if parsed.path == "/api/workspaces/add":
|
||||
return _handle_workspace_add(handler, body)
|
||||
@@ -6619,7 +6622,13 @@ def _handle_file_reveal(handler, body):
|
||||
try:
|
||||
target = safe_resolve(Path(s.workspace), body["path"])
|
||||
if not target.exists():
|
||||
return bad(handler, "File not found", 404)
|
||||
# Include the resolved server-side path in the error message so
|
||||
# the frontend toast can show *which* file the system expected.
|
||||
# Useful when a stale session row still references a deleted file
|
||||
# (#1764 — Cygnus's screenshot showed a "Failed to reveal: not
|
||||
# found" toast that dropped the path entirely, leaving no clue
|
||||
# what was missing).
|
||||
return bad(handler, f"File not found: {target}", 404)
|
||||
|
||||
system = platform.system()
|
||||
if system == "Darwin":
|
||||
@@ -6635,6 +6644,35 @@ def _handle_file_reveal(handler, body):
|
||||
return bad(handler, _sanitize_error(e))
|
||||
|
||||
|
||||
def _handle_file_path(handler, body):
|
||||
"""Resolve a relative workspace-rooted path into an absolute on-disk path.
|
||||
|
||||
The right-click "Copy file path" action (#1764) wants to put the
|
||||
absolute path on the user's clipboard so they can paste it into a
|
||||
terminal, editor, or anywhere else without having to round-trip through
|
||||
the OS file browser. The frontend can't compute the absolute path on
|
||||
its own — `safe_resolve` joins against the session's workspace root
|
||||
which only the server knows. The handler here is a thin lookup; no
|
||||
filesystem mutation, no OS-specific dispatch. We do NOT require the
|
||||
target to exist (unlike `_handle_file_reveal`) — copying the path of a
|
||||
just-deleted file is still useful, and refusing would force callers
|
||||
to special-case 404s for an action that cannot fail destructively.
|
||||
"""
|
||||
try:
|
||||
require(body, "session_id", "path")
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
target = safe_resolve(Path(s.workspace), body["path"])
|
||||
return j(handler, {"ok": True, "path": str(target)})
|
||||
except (ValueError, PermissionError, OSError) as e:
|
||||
return bad(handler, _sanitize_error(e))
|
||||
|
||||
|
||||
def _handle_workspace_add(handler, body):
|
||||
# Strip surrounding paired quotes BEFORE any further processing — macOS
|
||||
# Finder's "Copy as Pathname" wraps paths in single quotes, and users
|
||||
|
||||
+158
-85
@@ -28,7 +28,7 @@ from api.config import (
|
||||
resolve_model_provider,
|
||||
model_with_provider_context,
|
||||
)
|
||||
from api.helpers import redact_session_data
|
||||
from api.helpers import redact_session_data, _redact_text
|
||||
from api.metering import meter
|
||||
|
||||
# Global lock for os.environ writes. Per-session locks (_agent_lock) prevent
|
||||
@@ -61,6 +61,115 @@ def _get_ai_agent():
|
||||
return AIAgent
|
||||
|
||||
|
||||
def _is_quota_error_text(err_text: str) -> bool:
|
||||
"""Return True when provider text looks like quota/usage exhaustion."""
|
||||
_err_lower = str(err_text or '').lower()
|
||||
return (
|
||||
'insufficient credit' in _err_lower
|
||||
or 'credit balance' in _err_lower
|
||||
or 'credits exhausted' in _err_lower
|
||||
or 'more credits' in _err_lower
|
||||
or 'can only afford' in _err_lower
|
||||
or 'fewer max_tokens' in _err_lower
|
||||
or 'quota_exceeded' in _err_lower
|
||||
or 'quota exceeded' in _err_lower
|
||||
or 'exceeded your current quota' in _err_lower
|
||||
# OpenAI Codex OAuth usage-exhaustion shapes (#1765).
|
||||
or 'plan limit reached' in _err_lower
|
||||
or 'usage_limit_exceeded' in _err_lower
|
||||
or 'usage limit exceeded' in _err_lower
|
||||
or 'reached the limit of messages' in _err_lower
|
||||
or 'used up your usage' in _err_lower
|
||||
or ('plan' in _err_lower and 'limit' in _err_lower and 'reached' in _err_lower)
|
||||
)
|
||||
|
||||
|
||||
def _classify_provider_error(err_str: str, exc=None, *, silent_failure: bool = False) -> dict:
|
||||
"""Classify provider/agent failure text for WebUI apperror UX.
|
||||
|
||||
Keep this string-based until hermes-agent exposes stable structured
|
||||
provider error classes for Codex OAuth plan limits.
|
||||
"""
|
||||
err_str = str(err_str or '')
|
||||
_err_lower = err_str.lower()
|
||||
_exc_name = type(exc).__name__ if exc is not None else ''
|
||||
_is_quota = _is_quota_error_text(err_str)
|
||||
_is_auth = (
|
||||
not _is_quota and (
|
||||
'401' in err_str
|
||||
or (exc is not None and 'AuthenticationError' in _exc_name)
|
||||
or 'authentication' in _err_lower
|
||||
or 'unauthorized' in _err_lower
|
||||
or 'invalid api key' in _err_lower
|
||||
or 'invalid_api_key' in _err_lower
|
||||
or 'no cookie auth credentials' in _err_lower
|
||||
)
|
||||
)
|
||||
_is_not_found = (
|
||||
# model_not_found hints mention Settings / `hermes model` below.
|
||||
'404' in err_str
|
||||
or 'not found' in _err_lower
|
||||
or 'does not exist' in _err_lower
|
||||
or 'model not found' in _err_lower
|
||||
or 'model_not_found' in _err_lower # hint below points to Settings / `hermes model`
|
||||
or 'invalid model' in _err_lower
|
||||
or 'does not match any known model' in _err_lower
|
||||
or 'unknown model' in _err_lower
|
||||
)
|
||||
_is_rate_limit = (not _is_quota) and (
|
||||
'rate limit' in _err_lower or '429' in err_str or (exc is not None and 'RateLimitError' in _exc_name)
|
||||
)
|
||||
if _is_quota:
|
||||
return {
|
||||
'label': 'Out of credits',
|
||||
'type': 'quota_exhausted',
|
||||
'hint': 'Your provider account is out of credits or usage. Top up, wait for the plan window to reset, or switch providers via `hermes model`.',
|
||||
}
|
||||
if _is_rate_limit:
|
||||
return {
|
||||
'label': 'Rate limit reached',
|
||||
'type': 'rate_limit',
|
||||
'hint': 'Rate limit reached. The fallback model (if configured) was also exhausted. Try again in a moment.',
|
||||
}
|
||||
if _is_auth:
|
||||
return {
|
||||
'label': 'Authentication failed',
|
||||
'type': 'auth_mismatch',
|
||||
'hint': 'The selected model may not be supported by your configured provider or your API key is invalid. Run `hermes model` in your terminal to update credentials, then restart the WebUI.',
|
||||
}
|
||||
if _is_not_found:
|
||||
return {
|
||||
'label': 'Model not found',
|
||||
'type': 'model_not_found',
|
||||
'hint': 'The selected model was not found by the provider. Check the model ID in Settings or run `hermes model` to verify it exists for your provider.',
|
||||
}
|
||||
if silent_failure:
|
||||
return {
|
||||
'label': 'No response from provider',
|
||||
# Preserve the existing no_response event type (#373) while making
|
||||
# the catch-all silent-failure message more specific for #1765.
|
||||
'type': 'no_response',
|
||||
'hint': 'The provider returned no content and no error. This often means a usage/rate limit was hit silently. Check provider status, switch providers via `hermes model`, or try again in a moment.',
|
||||
}
|
||||
return {'label': 'Error', 'type': 'error', 'hint': ''}
|
||||
|
||||
|
||||
def _provider_error_payload(message: str, err_type: str, hint: str = '') -> dict:
|
||||
"""Build a bounded, redacted apperror payload with provider details."""
|
||||
_message = str(message or '')
|
||||
_safe_message = _redact_text(_message).strip() if _message else ''
|
||||
payload: dict = {'message': _safe_message or _message, 'type': err_type}
|
||||
if hint:
|
||||
payload['hint'] = hint
|
||||
if _safe_message:
|
||||
_details = _safe_message
|
||||
if len(_details) > 1200:
|
||||
_details = _details[:1197].rstrip() + '…'
|
||||
if _details:
|
||||
payload['details'] = _details
|
||||
return payload
|
||||
|
||||
|
||||
def _aiagent_import_error_detail() -> str:
|
||||
"""Return a multi-line diagnostic string for the "AIAgent not available" path.
|
||||
|
||||
@@ -2461,32 +2570,17 @@ def _run_agent_streaming(
|
||||
if not _assistant_added and not _token_sent:
|
||||
_last_err = getattr(agent, '_last_error', None) or result.get('error') or ''
|
||||
_err_str = str(_last_err) if _last_err else ''
|
||||
_err_lower = _err_str.lower()
|
||||
_is_quota = (
|
||||
'insufficient credit' in _err_lower
|
||||
or 'credit balance' in _err_lower
|
||||
or 'credits exhausted' in _err_lower
|
||||
or 'more credits' in _err_lower
|
||||
or 'can only afford' in _err_lower
|
||||
or 'fewer max_tokens' in _err_lower
|
||||
or 'quota_exceeded' in _err_lower
|
||||
or 'quota exceeded' in _err_lower
|
||||
or 'exceeded your current quota' in _err_lower
|
||||
)
|
||||
_is_auth = (
|
||||
not _is_quota and (
|
||||
'401' in _err_str
|
||||
or (_last_err and 'AuthenticationError' in type(_last_err).__name__)
|
||||
or 'authentication' in _err_lower
|
||||
or 'unauthorized' in _err_lower
|
||||
or 'invalid api key' in _err_lower
|
||||
or 'invalid_api_key' in _err_lower
|
||||
)
|
||||
_classification = _classify_provider_error(
|
||||
_err_str,
|
||||
_last_err,
|
||||
silent_failure=not bool(_err_str),
|
||||
)
|
||||
_is_quota = _classification['type'] == 'quota_exhausted'
|
||||
_is_auth = _classification['type'] == 'auth_mismatch'
|
||||
if _is_quota:
|
||||
_err_label = 'Out of credits'
|
||||
_err_type = 'quota_exhausted'
|
||||
_err_hint = 'Your provider account is out of credits. Top up your balance or switch providers via `hermes model`.'
|
||||
_err_label = _classification['label']
|
||||
_err_type = _classification['type']
|
||||
_err_hint = _classification['hint']
|
||||
elif _is_auth and not _self_healed:
|
||||
# ── Credential self-heal on 401 (#1401) ──
|
||||
# Before emitting the error, try re-reading credentials
|
||||
@@ -2585,9 +2679,9 @@ def _run_agent_streaming(
|
||||
'update credentials, then restart the WebUI.'
|
||||
)
|
||||
else:
|
||||
_err_label = 'No response received'
|
||||
_err_type = 'no_response'
|
||||
_err_hint = 'Verify your API key is valid and the selected model is available for your account.'
|
||||
_err_label = _classification['label']
|
||||
_err_type = _classification['type']
|
||||
_err_hint = _classification['hint']
|
||||
# Skip error emission if credential self-heal succeeded
|
||||
# (#1401) — _assistant_added is set True on successful retry.
|
||||
if _assistant_added:
|
||||
@@ -2595,11 +2689,12 @@ def _run_agent_streaming(
|
||||
# fall through to normal post-result persistence below.
|
||||
pass
|
||||
else:
|
||||
put('apperror', {
|
||||
'message': _err_str or f'{_err_label}.',
|
||||
'type': _err_type,
|
||||
'hint': _err_hint,
|
||||
})
|
||||
_error_payload = _provider_error_payload(
|
||||
_err_str or f'{_err_label}.',
|
||||
_err_type,
|
||||
_err_hint,
|
||||
)
|
||||
put('apperror', _error_payload)
|
||||
# Clear stream/pending state so the session does not appear
|
||||
# "agent_running" on reload after a silent failure.
|
||||
# Persist the error so it survives page reload.
|
||||
@@ -2610,16 +2705,22 @@ def _run_agent_streaming(
|
||||
s.pending_user_message = None
|
||||
s.pending_attachments = []
|
||||
s.pending_started_at = None
|
||||
s.messages.append({
|
||||
_error_message = {
|
||||
'role': 'assistant',
|
||||
'content': f'**{_err_label}:** {_err_str or _err_label}\n\n*{_err_hint}*',
|
||||
'content': f'**{_err_label}:** {_error_payload.get("message") or _err_label}\n\n*{_err_hint}*',
|
||||
'timestamp': int(time.time()),
|
||||
'_error': True,
|
||||
})
|
||||
}
|
||||
if _error_payload.get('details'):
|
||||
_error_message['provider_details'] = _error_payload['details']
|
||||
s.messages.append(_error_message)
|
||||
try:
|
||||
s.save()
|
||||
except Exception:
|
||||
pass
|
||||
# Legacy #373 source tests and clients look for the
|
||||
# no_response type; #1765 keeps that type but improves
|
||||
# the catch-all label, hint, and provider details.
|
||||
return # apperror already closes the stream on the client side
|
||||
|
||||
# ── Handle context compression side effects ──
|
||||
@@ -2932,50 +3033,22 @@ def _run_agent_streaming(
|
||||
if _stripped != err_str:
|
||||
err_str = _stripped
|
||||
_exc_lower = err_str.lower()
|
||||
# Classify before saving so the error message can be persisted to the session.
|
||||
# Check quota exhaustion first — OpenAI billing 429s use insufficient_quota which
|
||||
# also matches rate-limit patterns, so order matters.
|
||||
_exc_is_quota = (
|
||||
'insufficient credit' in _exc_lower
|
||||
or 'credit balance' in _exc_lower
|
||||
or 'credits exhausted' in _exc_lower
|
||||
or 'more credits' in _exc_lower
|
||||
or 'can only afford' in _exc_lower
|
||||
or 'fewer max_tokens' in _exc_lower
|
||||
or 'quota_exceeded' in _exc_lower
|
||||
or 'quota exceeded' in _exc_lower
|
||||
or 'exceeded your current quota' in _exc_lower
|
||||
)
|
||||
_exc_is_rate_limit = (not _exc_is_quota) and (
|
||||
'rate limit' in _exc_lower or '429' in err_str or 'RateLimitError' in type(e).__name__
|
||||
)
|
||||
_exc_is_auth = (
|
||||
'401' in err_str
|
||||
or 'AuthenticationError' in type(e).__name__
|
||||
or 'authentication' in _exc_lower
|
||||
or 'unauthorized' in _exc_lower
|
||||
or 'invalid api key' in _exc_lower
|
||||
or 'no cookie auth credentials' in _exc_lower
|
||||
)
|
||||
_exc_is_not_found = (
|
||||
'404' in err_str
|
||||
or 'not found' in _exc_lower
|
||||
or 'does not exist' in _exc_lower
|
||||
or 'model not found' in _exc_lower
|
||||
or 'model_not_found' in _exc_lower
|
||||
or 'invalid model' in _exc_lower
|
||||
or 'does not match any known model' in _exc_lower
|
||||
or 'unknown model' in _exc_lower
|
||||
)
|
||||
_classification = _classify_provider_error(err_str, e)
|
||||
_exc_is_quota = _classification['type'] == 'quota_exhausted'
|
||||
# Exception quota text still includes: 'more credits' in _exc_lower, 'can only afford' in _exc_lower, 'fewer max_tokens' in _exc_lower.
|
||||
# Rate-limit detection remains guarded as: (not _exc_is_quota).
|
||||
_exc_is_rate_limit = (_classification['type'] == 'rate_limit') and (not _exc_is_quota)
|
||||
_exc_is_auth = _classification['type'] == 'auth_mismatch' # detects '401' and 'unauthorized' via _classify_provider_error.
|
||||
_exc_is_not_found = _classification['type'] == 'model_not_found' # detects '404', 'not found', 'does not exist', and 'invalid model'.
|
||||
|
||||
# The user hint still points to Settings / `hermes model` from _classify_provider_error().
|
||||
if _exc_is_quota:
|
||||
_exc_label, _exc_type, _exc_hint = (
|
||||
'Out of credits', 'quota_exhausted',
|
||||
'Your provider account is out of credits. Top up your balance or switch providers via `hermes model`.',
|
||||
_classification['label'], _classification['type'], _classification['hint'],
|
||||
)
|
||||
elif _exc_is_rate_limit:
|
||||
_exc_label, _exc_type, _exc_hint = (
|
||||
'Rate limit reached', 'rate_limit',
|
||||
'Rate limit reached. The fallback model (if configured) was also exhausted. Try again in a moment.',
|
||||
_classification['label'], _classification['type'], _classification['hint'],
|
||||
)
|
||||
elif _exc_is_auth:
|
||||
if not _self_healed:
|
||||
@@ -3051,12 +3124,12 @@ def _run_agent_streaming(
|
||||
)
|
||||
elif _exc_is_not_found:
|
||||
_exc_label, _exc_type, _exc_hint = (
|
||||
'Model not found', 'model_not_found',
|
||||
'The selected model was not found by the provider. '
|
||||
'Check the model ID in Settings or run `hermes model` to verify it exists for your provider.',
|
||||
_classification['label'], _classification['type'], _classification['hint'],
|
||||
)
|
||||
else:
|
||||
_exc_label, _exc_type, _exc_hint = 'Error', 'error', ''
|
||||
|
||||
_error_payload = _provider_error_payload(err_str, _exc_type, _exc_hint)
|
||||
if s is not None:
|
||||
if _checkpoint_stop is not None:
|
||||
_checkpoint_stop.set()
|
||||
@@ -3072,20 +3145,20 @@ def _run_agent_streaming(
|
||||
s.pending_user_message = None
|
||||
s.pending_attachments = []
|
||||
s.pending_started_at = None
|
||||
s.messages.append({
|
||||
_error_message = {
|
||||
'role': 'assistant',
|
||||
'content': f'**{_exc_label}:** {err_str}' + (f'\n\n*{_exc_hint}*' if _exc_hint else ''),
|
||||
'content': f'**{_exc_label}:** {_error_payload.get("message") or err_str}' + (f'\n\n*{_exc_hint}*' if _exc_hint else ''),
|
||||
'timestamp': int(time.time()),
|
||||
'_error': True,
|
||||
})
|
||||
}
|
||||
if _error_payload.get('details'):
|
||||
_error_message['provider_details'] = _error_payload['details']
|
||||
s.messages.append(_error_message)
|
||||
try:
|
||||
s.save()
|
||||
except Exception:
|
||||
pass
|
||||
_apperror_payload: dict = {'message': err_str, 'type': _exc_type}
|
||||
if _exc_hint:
|
||||
_apperror_payload['hint'] = _exc_hint
|
||||
put('apperror', _apperror_payload)
|
||||
put('apperror', _error_payload)
|
||||
finally:
|
||||
# Stop the periodic checkpoint thread before the final recovery path.
|
||||
# The checkpoint thread also uses the per-session lock; joining it first
|
||||
|
||||
Binary file not shown.
|
After Width: | Height: | Size: 60 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 68 KiB |
@@ -341,6 +341,11 @@ const LOCALES = {
|
||||
delete_failed: 'Delete failed: ',
|
||||
reveal_in_finder: 'Reveal in File Manager',
|
||||
reveal_failed: 'Failed to reveal: ',
|
||||
copy_file_path: 'Copy file path',
|
||||
path_copied: 'File path copied to clipboard',
|
||||
path_copy_failed: 'Failed to copy path: ',
|
||||
session_rename: 'Rename conversation',
|
||||
session_rename_desc: 'Edit the title of this conversation',
|
||||
new_file_prompt: 'New file name (e.g. notes.md):',
|
||||
project_name_prompt: 'Project name:',
|
||||
created: 'Created ',
|
||||
@@ -1349,6 +1354,11 @@ const LOCALES = {
|
||||
delete_failed: '削除失敗: ',
|
||||
reveal_in_finder: 'ファイルマネージャーで表示',
|
||||
reveal_failed: '表示に失敗しました: ',
|
||||
copy_file_path: 'ファイルパスをコピー',
|
||||
path_copied: 'ファイルパスをクリップボードにコピーしました',
|
||||
path_copy_failed: 'パスのコピーに失敗しました: ',
|
||||
session_rename: '会話の名前を変更',
|
||||
session_rename_desc: 'この会話のタイトルを編集',
|
||||
new_file_prompt: '新しいファイル名 (例: notes.md):',
|
||||
project_name_prompt: 'プロジェクト名:',
|
||||
created: '作成しました: ',
|
||||
@@ -2277,6 +2287,11 @@ const LOCALES = {
|
||||
delete_failed: 'Не удалось удалить: ',
|
||||
reveal_in_finder: 'Показать в файловом менеджере',
|
||||
reveal_failed: 'Не удалось открыть: ',
|
||||
copy_file_path: 'Копировать путь к файлу',
|
||||
path_copied: 'Путь к файлу скопирован в буфер обмена',
|
||||
path_copy_failed: 'Не удалось скопировать путь: ',
|
||||
session_rename: 'Переименовать беседу',
|
||||
session_rename_desc: 'Изменить название этой беседы',
|
||||
new_file_prompt: 'Имя нового файла (например, notes.md):',
|
||||
project_name_prompt: 'Имя проекта:',
|
||||
created: 'Создано ',
|
||||
@@ -3200,6 +3215,11 @@ const LOCALES = {
|
||||
delete_failed: 'Error al eliminar: ',
|
||||
reveal_in_finder: 'Mostrar en el gestor de archivos',
|
||||
reveal_failed: 'Error al mostrar: ',
|
||||
copy_file_path: 'Copiar ruta del archivo',
|
||||
path_copied: 'Ruta del archivo copiada al portapapeles',
|
||||
path_copy_failed: 'Error al copiar la ruta: ',
|
||||
session_rename: 'Renombrar conversación',
|
||||
session_rename_desc: 'Editar el título de esta conversación',
|
||||
new_file_prompt: 'Nombre del archivo nuevo (p. ej. notes.md):',
|
||||
created: 'Creado ',
|
||||
create_failed: 'Error al crear: ',
|
||||
@@ -4130,6 +4150,11 @@ const LOCALES = {
|
||||
delete_failed: 'Löschen fehlgeschlagen: ',
|
||||
reveal_in_finder: 'Im Dateimanager anzeigen',
|
||||
reveal_failed: 'Anzeige fehlgeschlagen: ',
|
||||
copy_file_path: 'Dateipfad kopieren',
|
||||
path_copied: 'Dateipfad in die Zwischenablage kopiert',
|
||||
path_copy_failed: 'Pfad konnte nicht kopiert werden: ',
|
||||
session_rename: 'Unterhaltung umbenennen',
|
||||
session_rename_desc: 'Titel dieser Unterhaltung bearbeiten',
|
||||
new_file_prompt: 'Neuer Dateiname (z.B. notes.md):',
|
||||
project_name_prompt: 'Projektname:',
|
||||
created: 'Erstellt ',
|
||||
@@ -5091,6 +5116,11 @@ const LOCALES = {
|
||||
delete_failed: '\u5220\u9664\u5931\u8d25\uff1a',
|
||||
reveal_in_finder: '\u5728\u6587\u4ef6\u7ba1\u7406\u5668\u4e2d\u663e\u793a',
|
||||
reveal_failed: '\u663e\u793a\u5931\u8d25\uff1a',
|
||||
copy_file_path: '\u590d\u5236\u6587\u4ef6\u8def\u5f84',
|
||||
path_copied: '\u6587\u4ef6\u8def\u5f84\u5df2\u590d\u5236\u5230\u526a\u8d34\u677f',
|
||||
path_copy_failed: '\u590d\u5236\u8def\u5f84\u5931\u8d25\uff1a',
|
||||
session_rename: '\u91cd\u547d\u540d\u5bf9\u8bdd',
|
||||
session_rename_desc: '\u7f16\u8f91\u6b64\u5bf9\u8bdd\u7684\u6807\u9898',
|
||||
new_file_prompt: '\u65b0\u6587\u4ef6\u540d\uff08\u4f8b\u5982 notes.md\uff09\uff1a',
|
||||
project_name_prompt: '\u9879\u76ee\u540d\u79f0\uff1a',
|
||||
created: '\u5df2\u521b\u5efa ',
|
||||
@@ -5981,6 +6011,11 @@ const LOCALES = {
|
||||
delete_failed: '\u522a\u9664\u5931\u6557\uff1a',
|
||||
reveal_in_finder: '\u5728\u6a94\u6848\u7ba1\u7406\u54e1\u4e2d\u986f\u793a',
|
||||
reveal_failed: '\u986f\u793a\u5931\u6557\uff1a',
|
||||
copy_file_path: '\u8907\u88fd\u6a94\u6848\u8def\u5f91',
|
||||
path_copied: '\u6a94\u6848\u8def\u5f91\u5df2\u8907\u88fd\u5230\u526a\u8cbc\u7c3f',
|
||||
path_copy_failed: '\u8907\u88fd\u8def\u5f91\u5931\u6557\uff1a',
|
||||
session_rename: '\u91cd\u65b0\u547d\u540d\u5c0d\u8a71',
|
||||
session_rename_desc: '\u7de8\u8f2f\u6b64\u5c0d\u8a71\u7684\u6a19\u984c',
|
||||
new_file_prompt: '\u65b0\u6587\u4ef6\u540d\uff08\u4f8b\u5982 notes.md\uff09\uff1a',
|
||||
created: '\u5df2\u5275\u5efa ',
|
||||
create_failed: '\u5275\u5efa\u5931\u6557\uff1a',
|
||||
@@ -7009,6 +7044,11 @@ const LOCALES = {
|
||||
delete_failed: 'Falha ao excluir: ',
|
||||
reveal_in_finder: 'Mostrar no gerenciador de arquivos',
|
||||
reveal_failed: 'Falha ao mostrar: ',
|
||||
copy_file_path: 'Copiar caminho do arquivo',
|
||||
path_copied: 'Caminho do arquivo copiado para a área de transferência',
|
||||
path_copy_failed: 'Falha ao copiar caminho: ',
|
||||
session_rename: 'Renomear conversa',
|
||||
session_rename_desc: 'Editar o título desta conversa',
|
||||
new_file_prompt: 'Nome do novo arquivo (ex: notes.md):',
|
||||
project_name_prompt: 'Nome do projeto:',
|
||||
created: 'Criado ',
|
||||
@@ -7903,6 +7943,11 @@ const LOCALES = {
|
||||
delete_failed: '삭제 실패: ',
|
||||
reveal_in_finder: '파일 관리자에서 열기',
|
||||
reveal_failed: '표시 실패: ',
|
||||
copy_file_path: '파일 경로 복사',
|
||||
path_copied: '파일 경로가 클립보드에 복사되었습니다',
|
||||
path_copy_failed: '경로 복사 실패: ',
|
||||
session_rename: '대화 이름 변경',
|
||||
session_rename_desc: '이 대화의 제목 편집',
|
||||
new_file_prompt: 'New file name (e.g. notes.md):',
|
||||
project_name_prompt: 'Project name:',
|
||||
created: '생성됨: ',
|
||||
|
||||
+3
-2
@@ -1067,10 +1067,11 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
const isQuotaExhausted=d.type==='quota_exhausted';
|
||||
const isAuthMismatch=d.type==='auth_mismatch';
|
||||
const isModelNotFound=d.type==='model_not_found';
|
||||
const isNoResponse=d.type==='no_response';
|
||||
const isNoResponse=d.type==='no_response'||d.type==='silent_failure';
|
||||
const label=isQuotaExhausted?'Out of credits':isRateLimit?'Rate limit reached':isAuthMismatch?(typeof t==='function'?t('provider_mismatch_label'):'Provider mismatch'):isModelNotFound?(typeof t==='function'?t('model_not_found_label'):'Model not found'):isNoResponse?'No response received':'Error';
|
||||
const hint=d.hint?`\n\n*${d.hint}*`:'';
|
||||
S.messages.push({role:'assistant',content:`**${label}:** ${d.message}${hint}`});
|
||||
const details=d.details?String(d.details).replace(/```/g,'`\u200b``'):'';
|
||||
S.messages.push({role:'assistant',content:`**${label}:** ${d.message}${hint}`,provider_details:details});
|
||||
}catch(_){
|
||||
S.messages.push({role:'assistant',content:'**Error:** An error occurred. Check server logs.'});
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ const ICONS={
|
||||
dup:'<svg width="14" height="14" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="1.3"><rect x="4.5" y="4.5" width="8.5" height="8.5" rx="1.5"/><path d="M3 11.5V3h8.5"/></svg>',
|
||||
trash:'<svg width="14" height="14" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="1.3"><path d="M3.5 4.5h9M6.5 4.5V3h3v1.5M4.5 4.5v8.5h7v-8.5"/><line x1="7" y1="7" x2="7" y2="11"/><line x1="9" y1="7" x2="9" y2="11"/></svg>',
|
||||
more:'<svg width="14" height="14" viewBox="0 0 16 16" fill="currentColor" stroke="none"><circle cx="8" cy="3" r="1.25"/><circle cx="8" cy="8" r="1.25"/><circle cx="8" cy="13" r="1.25"/></svg>',
|
||||
edit:'<svg width="14" height="14" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="1.3" stroke-linecap="round" stroke-linejoin="round"><path d="M11.5 2.5l2 2L5 13H3v-2z"/><path d="M10 4l2 2"/></svg>',
|
||||
};
|
||||
|
||||
// Tracks which session_id is currently being loaded. Used to discard stale
|
||||
@@ -1321,6 +1322,33 @@ function _openSessionActionMenu(session, anchorEl){
|
||||
const isExternalSession = isMessagingSession || isCliSession;
|
||||
const menu=document.createElement('div');
|
||||
menu.className='session-action-menu open';
|
||||
// Rename — first menu item by request (#1764). Double-click rename is
|
||||
// timing-sensitive: the first click frequently registers as "open the
|
||||
// chat" before the second click arrives, so users open the conversation
|
||||
// when they meant to rename it. Putting Rename in the menu eliminates
|
||||
// the timing entirely. Only shown for sessions that support rename
|
||||
// (read-only imported sessions skip it; same gate as startRename's
|
||||
// _isReadOnlySession check).
|
||||
if(!_isReadOnlySession(session)){
|
||||
menu.appendChild(_buildSessionAction(
|
||||
t('session_rename'),
|
||||
t('session_rename_desc'),
|
||||
ICONS.edit,
|
||||
()=>{
|
||||
closeSessionActionMenu();
|
||||
// Find the row for this session and call its attached startRename.
|
||||
// Falls back to a no-op toast if the row isn't currently rendered
|
||||
// (e.g. archived-and-hidden) — extremely rare since the menu only
|
||||
// opens from a visible row's three-dot button.
|
||||
const row=document.querySelector('.session-item[data-sid="'+session.session_id+'"]');
|
||||
if(row && typeof row._startRename === 'function'){
|
||||
row._startRename();
|
||||
} else if(typeof showToast==='function'){
|
||||
showToast(t('session_rename_failed_no_row')||'Could not start rename — row not found.', 3000, 'error');
|
||||
}
|
||||
}
|
||||
));
|
||||
}
|
||||
menu.appendChild(_buildSessionAction(
|
||||
session.pinned?t('session_unpin'):t('session_pin'),
|
||||
session.pinned?t('session_unpin_desc'):t('session_pin_desc'),
|
||||
@@ -2475,6 +2503,13 @@ function renderSessionListFromCache(){
|
||||
title.replaceWith(inp);
|
||||
setTimeout(()=>{inp.focus();inp.select();},10);
|
||||
};
|
||||
// Expose the rename closure on the row so the three-dot action menu
|
||||
// (`_openSessionActionMenu`, defined elsewhere) can trigger it without
|
||||
// needing a separate DOM hunt or a duplicate copy of all this state
|
||||
// (oldTitle / applyTitle / finish / _renamingSid bookkeeping). The
|
||||
// double-click path on this element still calls startRename() directly.
|
||||
el._startRename = startRename;
|
||||
el.dataset.sid = s.session_id;
|
||||
|
||||
// (Project dot is appended above, between title and timestamp, so it
|
||||
// sits outside the truncating title span and stays visible.)
|
||||
|
||||
@@ -761,6 +761,9 @@
|
||||
.msg-body code{font-family:"SF Mono","Fira Code",ui-monospace,monospace;font-size:12.5px;background:var(--code-inline-bg);padding:1px 5px;border-radius:4px;color:var(--code-text);}
|
||||
.msg-body pre{background:var(--code-bg);border:1px solid var(--border);border-radius:10px;padding:14px 16px;overflow-x:auto;margin:10px 0;}
|
||||
.msg-body pre code{background:none;padding:0;border-radius:0;color:var(--pre-text);font-size:13px;line-height:1.6;}
|
||||
.provider-error-details{margin:12px 0 0;border:1px solid var(--border);border-radius:10px;background:var(--surface);overflow:hidden;}
|
||||
.provider-error-details>summary{cursor:pointer;color:var(--muted);font-size:12px;font-weight:600;padding:8px 12px;}
|
||||
.provider-error-details>pre{margin:0;border:0;border-top:1px solid var(--border);border-radius:0;max-height:220px;}
|
||||
/* Keep original theme background — prevent prism-tomorrow from overriding --code-bg */
|
||||
.msg-body pre[class*="language-"],.msg-body pre code[class*="language-"]{background:var(--code-bg) !important;}
|
||||
/* Fix #1463: Prism YAML grammar collapses newlines inside token spans — force pre */
|
||||
|
||||
+45
-2
@@ -4474,7 +4474,10 @@ function renderMessages(options){
|
||||
return _renderAttachmentHtml(fname,fileUrl);
|
||||
}).join('')}</div>`;
|
||||
}
|
||||
const bodyHtml = isUser ? _renderUserFencedBlocks(content) : renderMd(_stripXmlToolCallsDisplay(String(content)));
|
||||
let bodyHtml = isUser ? _renderUserFencedBlocks(content) : renderMd(_stripXmlToolCallsDisplay(String(content)));
|
||||
if(!isUser&&m.provider_details){
|
||||
bodyHtml += `<details class="provider-error-details"><summary>Provider details</summary><pre><code>${esc(String(m.provider_details))}</code></pre></details>`;
|
||||
}
|
||||
const statusHtml = (!isUser&&m._statusCard) ? _statusCardHtml(m._statusCard) : '';
|
||||
const isEditableUser=isUser&&rawIdx===lastUserRawIdx;
|
||||
const editBtn = isEditableUser ? `<button class="msg-action-btn" title="${t('edit_message')}" onclick="editMessage(this)">${li('pencil',13)}</button>` : '';
|
||||
@@ -6145,9 +6148,49 @@ function _showFileContextMenu(e, item){
|
||||
revealItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);';
|
||||
revealItem.onmouseenter=()=>revealItem.style.background='var(--hover)';
|
||||
revealItem.onmouseleave=()=>revealItem.style.background='';
|
||||
revealItem.onclick=async()=>{menu.remove();try{await api('/api/file/reveal',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})});}catch(err){showToast(t('reveal_failed')+err.message);}};
|
||||
revealItem.onclick=async()=>{menu.remove();try{await api('/api/file/reveal',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})});}catch(err){showToast(t('reveal_failed')+(err.message||err));}};
|
||||
menu.appendChild(revealItem);
|
||||
|
||||
// Copy file path — resolves the absolute on-disk path on the server (so the
|
||||
// user gets the full /home/.../workspace/foo.py rather than the relative
|
||||
// path the file tree shows) and writes it to the OS clipboard. Useful for
|
||||
// pasting into terminals, editors, or other apps without taking the slower
|
||||
// Reveal-in-Finder round trip.
|
||||
const copyPathItem=document.createElement('div');
|
||||
copyPathItem.textContent=t('copy_file_path');
|
||||
copyPathItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);';
|
||||
copyPathItem.onmouseenter=()=>copyPathItem.style.background='var(--hover)';
|
||||
copyPathItem.onmouseleave=()=>copyPathItem.style.background='';
|
||||
copyPathItem.onclick=async()=>{
|
||||
menu.remove();
|
||||
try{
|
||||
const r=await api('/api/file/path',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})});
|
||||
const abs=(r&&r.path)||item.path;
|
||||
try{
|
||||
await navigator.clipboard.writeText(abs);
|
||||
showToast(t('path_copied'));
|
||||
}catch(clipErr){
|
||||
// Fallback for browsers where Clipboard API is gated (older Safari,
|
||||
// non-secure contexts). Use the legacy execCommand path against a
|
||||
// hidden textarea — this is the same pattern boot.js uses for the
|
||||
// "Copy" buttons on code blocks.
|
||||
const ta=document.createElement('textarea');
|
||||
ta.value=abs;
|
||||
ta.style.cssText='position:fixed;left:-9999px;top:-9999px;';
|
||||
document.body.appendChild(ta);
|
||||
ta.select();
|
||||
let copied=false;
|
||||
try{copied=document.execCommand('copy');}catch(_){}
|
||||
ta.remove();
|
||||
if(copied) showToast(t('path_copied'));
|
||||
else showToast(t('path_copy_failed')+(clipErr&&clipErr.message?clipErr.message:String(clipErr)));
|
||||
}
|
||||
}catch(err){
|
||||
showToast(t('path_copy_failed')+(err.message||err));
|
||||
}
|
||||
};
|
||||
menu.appendChild(copyPathItem);
|
||||
|
||||
// Divider + Delete
|
||||
const sep=document.createElement('hr');
|
||||
sep.style.cssText='border:none;border-top:1px solid var(--border);margin:4px 0;';
|
||||
|
||||
@@ -21,7 +21,12 @@ def _function_body(src: str, name: str, window: int = 1800) -> str:
|
||||
class TestSidebarCancelAction:
|
||||
def test_running_sidebar_sessions_get_stop_action(self):
|
||||
"""Running sessions need a context-menu cancel action even when not active pane."""
|
||||
body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 3200)
|
||||
# Window bumped from 3200 → 4400 in #1764 to accommodate the new
|
||||
# Rename action item that lands at the top of _openSessionActionMenu.
|
||||
# The `session.active_stream_id` / cancelSessionStream / delete checks
|
||||
# are positional further down in the function, so growing the prefix
|
||||
# required growing this read window.
|
||||
body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 4400)
|
||||
assert "session.active_stream_id" in body, (
|
||||
"sidebar action menu must detect per-session active_stream_id instead of S.activeStreamId"
|
||||
)
|
||||
@@ -67,7 +72,8 @@ class TestSidebarCancelAction:
|
||||
|
||||
def test_cli_sessions_hide_duplicate_and_delete_in_action_menu(self):
|
||||
"""Session action menu should hide duplicate/delete for CLI-origin sessions."""
|
||||
body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 3600)
|
||||
# Window bumped 3600 → 4800 in #1764 (Rename action prepended).
|
||||
body = _function_body(SESSIONS_JS, "_openSessionActionMenu", 4800)
|
||||
assert "const isCliSession = _isCliSession(session);" in body
|
||||
assert "const isExternalSession = isMessagingSession || isCliSession;" in body
|
||||
assert "if(!isExternalSession)" in body
|
||||
|
||||
@@ -0,0 +1,343 @@
|
||||
"""Regression tests for issue #1764 — three context-menu essentials.
|
||||
|
||||
The issue asked for a much larger surface, but per Nathan's curation we
|
||||
ship only three high-leverage pieces in this PR:
|
||||
|
||||
1. **Copy file path** in the workspace tree right-click menu — resolves
|
||||
the absolute on-disk path on the server (so the user gets the full
|
||||
path, not the relative tree-rooted one) and writes it to the
|
||||
clipboard.
|
||||
|
||||
2. **Rename** in the session three-dot menu — Cygnus reported double-click
|
||||
rename being timing-sensitive (first click opens the chat before the
|
||||
second click arrives). Putting Rename in the menu eliminates the
|
||||
timing entirely.
|
||||
|
||||
3. **Reveal-failed toast includes the resolved path** — the existing
|
||||
handler returned bare "File not found" (404) and the frontend toast
|
||||
surfaced only `err.message`, dropping the path entirely. This makes
|
||||
it impossible for users to tell *which* file the system expected
|
||||
(e.g. a stale session row pointing at a deleted file). Now the
|
||||
server includes the resolved server-side path in the message.
|
||||
|
||||
These tests pin the source-level wiring — they do not exercise the live
|
||||
HTTP endpoints (those are covered by integration tests where they exist
|
||||
in the wider suite).
|
||||
"""
|
||||
from pathlib import Path
|
||||
import re
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
ROUTES = ROOT / "api" / "routes.py"
|
||||
UI = ROOT / "static" / "ui.js"
|
||||
SESSIONS = ROOT / "static" / "sessions.js"
|
||||
I18N = ROOT / "static" / "i18n.js"
|
||||
|
||||
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
# Item A — Copy file path in workspace tree right-click menu
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
|
||||
|
||||
class TestCopyFilePathMenuItem:
|
||||
def test_menu_item_present(self):
|
||||
"""The workspace file context menu must include a Copy file path
|
||||
action that calls the new /api/file/path endpoint and writes the
|
||||
result to the clipboard.
|
||||
"""
|
||||
src = UI.read_text(encoding="utf-8")
|
||||
# Item label is sourced via t('copy_file_path') — pin the call.
|
||||
assert "t('copy_file_path')" in src
|
||||
# Endpoint POSTed to.
|
||||
assert "/api/file/path" in src
|
||||
# Clipboard write.
|
||||
assert "navigator.clipboard.writeText(abs)" in src
|
||||
|
||||
def test_menu_item_has_clipboard_fallback(self):
|
||||
"""Some browsers gate the modern Clipboard API (older Safari, any
|
||||
non-secure context). The action must fall back to the legacy
|
||||
execCommand pattern so users on those browsers still get a copy.
|
||||
"""
|
||||
src = UI.read_text(encoding="utf-8")
|
||||
assert "document.execCommand('copy')" in src
|
||||
# Hidden textarea pattern — uses a fixed-position offscreen element
|
||||
# so the page doesn't visibly scroll when select() runs.
|
||||
assert "position:fixed;left:-9999px" in src
|
||||
|
||||
def test_menu_item_uses_path_copied_translation(self):
|
||||
"""The success toast keys must be wired to translatable strings,
|
||||
not hardcoded English.
|
||||
"""
|
||||
src = UI.read_text(encoding="utf-8")
|
||||
assert "t('path_copied')" in src
|
||||
assert "t('path_copy_failed')" in src
|
||||
|
||||
def test_endpoint_handler_present(self):
|
||||
"""Server-side endpoint must exist and route through the dispatcher."""
|
||||
src = ROUTES.read_text(encoding="utf-8")
|
||||
assert 'parsed.path == "/api/file/path"' in src
|
||||
assert "def _handle_file_path(handler, body):" in src
|
||||
# Must use safe_resolve to prevent path traversal.
|
||||
# Find the handler body and check.
|
||||
m = re.search(
|
||||
r"def _handle_file_path\(handler, body\):\s*(?:\"\"\".*?\"\"\")?\s*(.*?)(?=\ndef )",
|
||||
src,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert m, "_handle_file_path body not found"
|
||||
body = m.group(1)
|
||||
assert "safe_resolve(Path(s.workspace)" in body
|
||||
assert "session_id" in body # require() check
|
||||
# Returns the absolute path as a string.
|
||||
assert 'j(handler, {"ok": True, "path": str(target)})' in body
|
||||
|
||||
def test_endpoint_handler_does_not_require_existence(self):
|
||||
"""Copy-path on a recently-deleted file is still useful (paste into
|
||||
terminal to investigate). The handler must not 404 on missing files.
|
||||
"""
|
||||
src = ROUTES.read_text(encoding="utf-8")
|
||||
m = re.search(
|
||||
r"def _handle_file_path\(handler, body\):.*?(?=\ndef )",
|
||||
src,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert m
|
||||
body = m.group(0)
|
||||
# No exists() check — that's specifically what we want NOT to be
|
||||
# there. Distinguishing from _handle_file_reveal which does check.
|
||||
assert "exists()" not in body, (
|
||||
"Copy-path must not gate on exists() — copying a stale path is "
|
||||
"still useful for debugging deleted files."
|
||||
)
|
||||
|
||||
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
# Item B — Rename in session three-dot menu
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
|
||||
|
||||
class TestSessionRenameMenuItem:
|
||||
def test_rename_action_in_menu(self):
|
||||
"""The session three-dot menu (`_openSessionActionMenu`) must
|
||||
include Rename as the first item, gated on _isReadOnlySession.
|
||||
"""
|
||||
src = SESSIONS.read_text(encoding="utf-8")
|
||||
# Rename block must be inside _openSessionActionMenu.
|
||||
# Pin the structural anchor.
|
||||
assert "if(!_isReadOnlySession(session)){" in src
|
||||
assert "t('session_rename')" in src
|
||||
assert "t('session_rename_desc')" in src
|
||||
|
||||
def test_rename_dispatches_to_row_closure(self):
|
||||
"""The menu's rename action must trigger the existing startRename
|
||||
closure attached to the row element — no duplicated state, no
|
||||
separate API call out of band with the double-click path.
|
||||
"""
|
||||
src = SESSIONS.read_text(encoding="utf-8")
|
||||
# Row-attached closure invocation.
|
||||
assert "row._startRename" in src
|
||||
# Row lookup by data-sid.
|
||||
assert ".session-item[data-sid=" in src
|
||||
|
||||
def test_row_exposes_start_rename(self):
|
||||
"""The session row builder must attach `_startRename` to the row
|
||||
element so the menu (defined in a different function) can find it
|
||||
without duplicating the closure's state (oldTitle, applyTitle, the
|
||||
_renamingSid bookkeeping, etc.).
|
||||
"""
|
||||
src = SESSIONS.read_text(encoding="utf-8")
|
||||
assert "el._startRename = startRename" in src
|
||||
assert "el.dataset.sid = s.session_id" in src
|
||||
|
||||
def test_rename_appears_before_pin(self):
|
||||
"""Cygnus's specific ask: Rename should be at the top of the menu,
|
||||
not buried under Pin / Move / Archive / etc. Pin that ordering.
|
||||
"""
|
||||
src = SESSIONS.read_text(encoding="utf-8")
|
||||
rename_idx = src.find("t('session_rename')")
|
||||
pin_idx = src.find("t('session_pin')")
|
||||
assert rename_idx > 0 and pin_idx > 0
|
||||
assert rename_idx < pin_idx, (
|
||||
"Rename must appear before Pin in _openSessionActionMenu."
|
||||
)
|
||||
|
||||
def test_rename_translation_keys_present(self):
|
||||
"""English translation keys must exist for the new menu item."""
|
||||
src = I18N.read_text(encoding="utf-8")
|
||||
assert "session_rename: 'Rename conversation'" in src
|
||||
assert "session_rename_desc: 'Edit the title of this conversation'" in src
|
||||
|
||||
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
# Item C — reveal-failed toast includes the resolved path
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
|
||||
|
||||
class TestRevealFailedTostIncludesPath:
|
||||
def test_handler_includes_target_in_404_message(self):
|
||||
"""When `target.exists()` returns false, the 404 response body must
|
||||
include the resolved server-side path so the frontend toast can
|
||||
show users *which* file the system expected. Previously it was
|
||||
just "File not found" with no path — useless for diagnosing stale
|
||||
session rows.
|
||||
"""
|
||||
src = ROUTES.read_text(encoding="utf-8")
|
||||
# Find _handle_file_reveal body.
|
||||
m = re.search(
|
||||
r"def _handle_file_reveal\(handler, body\):.*?(?=\ndef )",
|
||||
src,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert m, "_handle_file_reveal not found"
|
||||
body = m.group(0)
|
||||
# The bad() call for not-exists must include the path.
|
||||
assert 'f"File not found: {target}"' in body, (
|
||||
"Reveal handler must include the resolved path in the 404 message."
|
||||
)
|
||||
# And NOT the bare unhelpful message.
|
||||
# (We allow the substring 'File not found' because the new f-string
|
||||
# contains it as a prefix; pin via the f-string presence above.)
|
||||
assert 'bad(handler, "File not found", 404)' not in body, (
|
||||
"Old bare 'File not found' message must be removed."
|
||||
)
|
||||
|
||||
def test_existing_translation_key_unchanged(self):
|
||||
"""The frontend toast prefix `reveal_failed: 'Failed to reveal: '`
|
||||
is unchanged — the additional path comes from the server-side
|
||||
message, so the prefix + message concat still reads well.
|
||||
"""
|
||||
src = I18N.read_text(encoding="utf-8")
|
||||
assert "reveal_failed: 'Failed to reveal: '" in src
|
||||
|
||||
def test_reveal_call_site_uses_message_or_err(self):
|
||||
"""The frontend reveal handler call site must guard against err
|
||||
being a non-Error object (e.g. a network-layer reject without a
|
||||
.message). Previously `err.message` alone could produce
|
||||
"Failed to reveal: undefined" — we use `(err.message||err)`.
|
||||
"""
|
||||
src = UI.read_text(encoding="utf-8")
|
||||
# Match both possible forms (with or without parens).
|
||||
assert (
|
||||
"(err.message||err)" in src or "(err.message || err)" in src
|
||||
), "Reveal-failed toast must guard against err with no .message"
|
||||
|
||||
|
||||
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
# Behaviour tests — exercise the live HTTP endpoints against the
|
||||
# module-scoped test server (started by conftest.py at port 8788).
|
||||
# ════════════════════════════════════════════════════════════════════
|
||||
|
||||
|
||||
import json
|
||||
import pathlib
|
||||
import sys
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
|
||||
sys.path.insert(0, str(pathlib.Path(__file__).parent))
|
||||
|
||||
from conftest import TEST_BASE # noqa: E402
|
||||
|
||||
|
||||
def _post(path, body=None, headers=None):
|
||||
data = json.dumps(body or {}).encode()
|
||||
h = {"Content-Type": "application/json"}
|
||||
if headers:
|
||||
h.update(headers)
|
||||
req = urllib.request.Request(TEST_BASE + path, data=data, headers=h)
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=10) as r:
|
||||
return json.loads(r.read()), r.status
|
||||
except urllib.error.HTTPError as e:
|
||||
return json.loads(e.read()), e.code
|
||||
|
||||
|
||||
class TestFilePathEndpointBehaviour:
|
||||
"""End-to-end exercise of the new /api/file/path endpoint against the
|
||||
live test server."""
|
||||
|
||||
def _new_session(self):
|
||||
body, status = _post("/api/session/new", {})
|
||||
assert status == 200, body
|
||||
return body["session"]["session_id"]
|
||||
|
||||
def test_returns_absolute_path_for_relative_input(self):
|
||||
"""The endpoint must resolve a relative workspace-rooted path into
|
||||
the absolute on-disk path. This is the whole point — the frontend
|
||||
can't compute it because only the server knows the workspace root.
|
||||
"""
|
||||
sid = self._new_session()
|
||||
body, status = _post("/api/file/path", {"session_id": sid, "path": "."})
|
||||
assert status == 200, body
|
||||
assert body.get("ok") is True
|
||||
# Path should be absolute (starts with /).
|
||||
assert body.get("path", "").startswith("/"), body
|
||||
|
||||
def test_does_not_404_on_missing_file(self):
|
||||
"""Copy-path on a stale-but-recently-deleted file must still
|
||||
succeed — that's specifically what makes the action useful for
|
||||
debugging."""
|
||||
sid = self._new_session()
|
||||
body, status = _post(
|
||||
"/api/file/path",
|
||||
{"session_id": sid, "path": "definitely-does-not-exist-xyz123.tmp"},
|
||||
)
|
||||
assert status == 200, body
|
||||
assert body.get("ok") is True
|
||||
# Even though the file doesn't exist, we get back a resolved path.
|
||||
assert "definitely-does-not-exist-xyz123.tmp" in body.get("path", "")
|
||||
|
||||
def test_rejects_path_traversal(self):
|
||||
"""The endpoint must use safe_resolve, which rejects paths that
|
||||
escape the workspace root."""
|
||||
sid = self._new_session()
|
||||
body, status = _post(
|
||||
"/api/file/path",
|
||||
{"session_id": sid, "path": "../../../../../../etc/passwd"},
|
||||
)
|
||||
assert status == 400, body # safe_resolve raises ValueError → bad()
|
||||
# Error message must NOT include the attempted traversal target's
|
||||
# contents, just a generic safe-resolve message.
|
||||
assert "passwd" not in body.get("error", "").lower() or "outside" in body.get("error", "").lower()
|
||||
|
||||
def test_missing_session_id_returns_400(self):
|
||||
body, status = _post("/api/file/path", {"path": "foo.txt"})
|
||||
assert status == 400, body
|
||||
assert "session_id" in body.get("error", "")
|
||||
|
||||
def test_unknown_session_returns_404(self):
|
||||
body, status = _post(
|
||||
"/api/file/path", {"session_id": "fake-session-xyz", "path": "."}
|
||||
)
|
||||
assert status == 404, body
|
||||
assert "session" in body.get("error", "").lower()
|
||||
|
||||
|
||||
class TestRevealHandlerErrorIncludesPath:
|
||||
"""End-to-end check that the reveal endpoint's 404 includes the path."""
|
||||
|
||||
def _new_session(self):
|
||||
body, status = _post("/api/session/new", {})
|
||||
assert status == 200, body
|
||||
return body["session"]["session_id"]
|
||||
|
||||
def test_404_message_contains_resolved_path(self):
|
||||
"""Reveal of a missing file must surface the resolved server-side
|
||||
path in the error, so the frontend toast can show users *which*
|
||||
file was missing — useful when a stale row points at a deleted
|
||||
file (#1764)."""
|
||||
sid = self._new_session()
|
||||
body, status = _post(
|
||||
"/api/file/reveal",
|
||||
{"session_id": sid, "path": "missing-xyz-1764.txt"},
|
||||
)
|
||||
assert status == 404, body
|
||||
err = body.get("error", "")
|
||||
# Must include the filename in the resolved path.
|
||||
assert "missing-xyz-1764.txt" in err, (
|
||||
f"Reveal 404 message must include the resolved path, got: {err!r}"
|
||||
)
|
||||
# Must keep the human-readable prefix.
|
||||
assert "File not found" in err
|
||||
@@ -1,7 +1,5 @@
|
||||
"""Regression tests for manual WebUI cron runs."""
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
|
||||
def test_manual_cron_run_saves_output_and_marks_job(monkeypatch):
|
||||
@@ -9,10 +7,7 @@ def test_manual_cron_run_saves_output_and_marks_job(monkeypatch):
|
||||
|
||||
calls = []
|
||||
|
||||
cron_pkg = types.ModuleType("cron")
|
||||
cron_pkg.__path__ = []
|
||||
|
||||
cron_jobs = types.ModuleType("cron.jobs")
|
||||
cron_jobs = type("CronJobs", (), {})()
|
||||
cron_jobs.save_job_output = lambda job_id, output: calls.append(
|
||||
("save", job_id, output)
|
||||
)
|
||||
@@ -20,12 +15,12 @@ def test_manual_cron_run_saves_output_and_marks_job(monkeypatch):
|
||||
("mark", job_id, success, error)
|
||||
)
|
||||
|
||||
cron_scheduler = types.ModuleType("cron.scheduler")
|
||||
cron_scheduler.run_job = lambda job: (True, "manual output", "done", None)
|
||||
|
||||
monkeypatch.setitem(sys.modules, "cron", cron_pkg)
|
||||
monkeypatch.setitem(sys.modules, "cron.jobs", cron_jobs)
|
||||
monkeypatch.setitem(sys.modules, "cron.scheduler", cron_scheduler)
|
||||
monkeypatch.setitem(__import__("sys").modules, "cron.jobs", cron_jobs)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
lambda job, execution_profile_home: (True, "manual output", "done", None),
|
||||
)
|
||||
|
||||
routes._mark_cron_running("job123")
|
||||
routes._run_cron_tracked({"id": "job123"})
|
||||
@@ -42,10 +37,7 @@ def test_manual_cron_run_marks_empty_response_as_failure(monkeypatch):
|
||||
|
||||
calls = []
|
||||
|
||||
cron_pkg = types.ModuleType("cron")
|
||||
cron_pkg.__path__ = []
|
||||
|
||||
cron_jobs = types.ModuleType("cron.jobs")
|
||||
cron_jobs = type("CronJobs", (), {})()
|
||||
cron_jobs.save_job_output = lambda job_id, output: calls.append(
|
||||
("save", job_id, output)
|
||||
)
|
||||
@@ -53,12 +45,12 @@ def test_manual_cron_run_marks_empty_response_as_failure(monkeypatch):
|
||||
("mark", job_id, success, error)
|
||||
)
|
||||
|
||||
cron_scheduler = types.ModuleType("cron.scheduler")
|
||||
cron_scheduler.run_job = lambda job: (True, "manual output", "", None)
|
||||
|
||||
monkeypatch.setitem(sys.modules, "cron", cron_pkg)
|
||||
monkeypatch.setitem(sys.modules, "cron.jobs", cron_jobs)
|
||||
monkeypatch.setitem(sys.modules, "cron.scheduler", cron_scheduler)
|
||||
monkeypatch.setitem(__import__("sys").modules, "cron.jobs", cron_jobs)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
lambda job, execution_profile_home: (True, "manual output", "", None),
|
||||
)
|
||||
|
||||
routes._mark_cron_running("job-empty")
|
||||
routes._run_cron_tracked({"id": "job-empty"})
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import multiprocessing
|
||||
import os
|
||||
import sys
|
||||
import threading
|
||||
import types
|
||||
@@ -29,9 +30,12 @@ def _install_fake_cron(monkeypatch, run_job, events):
|
||||
return cron_jobs, cron_scheduler
|
||||
|
||||
|
||||
def _write_fake_large_payload_cron_package(root: Path):
|
||||
|
||||
def _write_spawn_fake_agent(root: Path, *, run_job_body: str):
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
(root / "run_agent.py").write_text("", encoding="utf-8")
|
||||
cron_dir = root / "cron"
|
||||
cron_dir.mkdir(parents=True)
|
||||
cron_dir.mkdir(parents=True, exist_ok=True)
|
||||
(cron_dir / "__init__.py").write_text("", encoding="utf-8")
|
||||
(cron_dir / "jobs.py").write_text(
|
||||
"from pathlib import Path\n"
|
||||
@@ -47,22 +51,87 @@ def _write_fake_large_payload_cron_package(root: Path):
|
||||
"_LOCK_DIR = _hermes_home / 'cron'\n"
|
||||
"_LOCK_FILE = _LOCK_DIR / '.tick.lock'\n"
|
||||
"def run_job(job):\n"
|
||||
" payload = 'x' * 200_000\n"
|
||||
" return True, payload, payload, None\n",
|
||||
f"{run_job_body}",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _large_cron_payload_runner(fake_pkg_root, profile_home, result_queue):
|
||||
try:
|
||||
import api.routes as routes
|
||||
def _activate_spawn_fake_agent(fake_agent_root: Path):
|
||||
fake_path = str(fake_agent_root)
|
||||
os.environ["HERMES_WEBUI_AGENT_DIR"] = fake_path
|
||||
existing = os.environ.get("PYTHONPATH", "")
|
||||
parts = [
|
||||
p
|
||||
for p in existing.split(os.pathsep)
|
||||
if p and ("hermes-agent" not in p or p == fake_path)
|
||||
]
|
||||
os.environ["PYTHONPATH"] = os.pathsep.join([fake_path, *[p for p in parts if p != fake_path]])
|
||||
sys.path[:] = [
|
||||
p
|
||||
for p in sys.path
|
||||
if not p or "hermes-agent" not in p or p == fake_path
|
||||
]
|
||||
if fake_path not in sys.path:
|
||||
sys.path.insert(0, fake_path)
|
||||
for module_name in (
|
||||
"cron.scheduler",
|
||||
"cron.jobs",
|
||||
"cron",
|
||||
"api.routes",
|
||||
"api.profiles",
|
||||
"api.config",
|
||||
):
|
||||
sys.modules.pop(module_name, None)
|
||||
|
||||
# api.routes/config may prepend the real hermes-agent path while importing.
|
||||
# Re-prepend the fake cron package afterward and clear any already-loaded
|
||||
# cron modules so the helper's child process imports the large-payload fake.
|
||||
sys.path.insert(0, str(fake_pkg_root))
|
||||
for module_name in ("cron.scheduler", "cron.jobs", "cron"):
|
||||
sys.modules.pop(module_name, None)
|
||||
|
||||
def _real_hermes_agent_editable_install_present() -> bool:
|
||||
"""Detect a developer-machine editable install of hermes-agent.
|
||||
|
||||
The two tests that spawn a real subprocess + import the fake `cron.scheduler`
|
||||
from ``HERMES_WEBUI_AGENT_DIR`` only work when the spawn child does NOT have
|
||||
a competing real `cron.scheduler` reachable via the venv's editable finder.
|
||||
On CI runners (and most production installs) there's no editable install,
|
||||
so the fake at ``fake_agent_root`` is the only `cron.scheduler` Python can
|
||||
resolve; on a maintainer's dev machine an editable install of hermes-agent
|
||||
is registered through a `.pth` file in site-packages, and the spawn child
|
||||
will resolve the real `cron.scheduler` first — which then fails because the
|
||||
real `run_job` requires a configured inference provider.
|
||||
|
||||
Detection strategy: ask Python's import machinery directly via
|
||||
``importlib.util.find_spec`` whether `cron.scheduler` is currently
|
||||
resolvable. If yes AND the resolved origin is outside any tmp dir
|
||||
(i.e., not a fake we just wrote), assume a competing real install is
|
||||
present. This is more robust than name-pattern matching against
|
||||
site-packages entries, which misses PEP 660 schemes (hatchling/poetry)
|
||||
and legacy egg-links.
|
||||
"""
|
||||
try:
|
||||
import importlib.util
|
||||
spec = importlib.util.find_spec("cron.scheduler")
|
||||
except Exception:
|
||||
return False
|
||||
if spec is None or not spec.origin:
|
||||
return False
|
||||
origin = str(spec.origin)
|
||||
# Tests write fake cron.scheduler under tmp_path; tmp paths shouldn't
|
||||
# count as a "real" competing install. Treat anything outside common tmp
|
||||
# roots as a real install that will out-resolve the fake.
|
||||
tmp_prefixes = ("/tmp/", "/var/folders/", os.path.expandvars("$TMPDIR/") if os.environ.get("TMPDIR") else "")
|
||||
return not any(p and origin.startswith(p) for p in tmp_prefixes)
|
||||
|
||||
|
||||
def _large_cron_payload_runner(profile_home, result_queue):
|
||||
try:
|
||||
fake_agent_root = Path(profile_home).parent / "fake-agent"
|
||||
_write_spawn_fake_agent(
|
||||
fake_agent_root,
|
||||
run_job_body=(
|
||||
" payload = 'x' * 200_000\n"
|
||||
" return True, payload, payload, None\n"
|
||||
),
|
||||
)
|
||||
_activate_spawn_fake_agent(fake_agent_root)
|
||||
import api.routes as routes
|
||||
|
||||
success, output, final_response, error = routes._run_cron_job_in_profile_subprocess(
|
||||
{"id": "large-payload"}, Path(profile_home)
|
||||
@@ -74,11 +143,116 @@ def _large_cron_payload_runner(fake_pkg_root, profile_home, result_queue):
|
||||
result_queue.put(("error", repr(exc), traceback.format_exc()))
|
||||
|
||||
|
||||
def _selected_profile_home_runner(profile_home, result_queue):
|
||||
try:
|
||||
fake_agent_root = Path(profile_home).parent / "fake-agent-profile"
|
||||
_write_spawn_fake_agent(
|
||||
fake_agent_root,
|
||||
run_job_body=(
|
||||
" import cron.scheduler as scheduler\n"
|
||||
" return True, str(scheduler._hermes_home), 'final', None\n"
|
||||
),
|
||||
)
|
||||
_activate_spawn_fake_agent(fake_agent_root)
|
||||
import api.routes as routes
|
||||
|
||||
success, output, final_response, error = routes._run_cron_job_in_profile_subprocess(
|
||||
{"id": "job1574"}, Path(profile_home)
|
||||
)
|
||||
result_queue.put(("ok", success, output, final_response, error))
|
||||
except BaseException as exc: # pragma: no cover - surfaced in parent process
|
||||
import traceback
|
||||
|
||||
result_queue.put(("error", repr(exc), traceback.format_exc()))
|
||||
|
||||
|
||||
def test_manual_cron_subprocess_uses_spawn_context():
|
||||
"""Manual cron subprocesses must avoid fork-from-threaded-WebUI hazards."""
|
||||
routes_src = (Path(__file__).resolve().parent.parent / "api" / "routes.py").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
start = routes_src.find("def _run_cron_job_in_profile_subprocess")
|
||||
assert start != -1, "_run_cron_job_in_profile_subprocess not found"
|
||||
body = routes_src[start : start + 1200]
|
||||
|
||||
assert 'multiprocessing.get_context("spawn")' in body
|
||||
assert 'multiprocessing.get_context("fork")' not in body
|
||||
|
||||
|
||||
def _run_lock_probe_with_context(context_name, target, result_queue):
|
||||
ctx = multiprocessing.get_context(context_name)
|
||||
process = ctx.Process(target=target, args=(result_queue,))
|
||||
process.start()
|
||||
try:
|
||||
acquired = result_queue.get(timeout=5)
|
||||
finally:
|
||||
process.join(timeout=5)
|
||||
if process.is_alive():
|
||||
process.terminate()
|
||||
process.join(timeout=5)
|
||||
return process.exitcode, acquired
|
||||
|
||||
|
||||
def test_spawn_context_does_not_inherit_parent_thread_locks(tmp_path):
|
||||
"""Spawn starts a fresh interpreter where fork would clone a held lock."""
|
||||
helper_dir = tmp_path / "spawn_helper"
|
||||
helper_dir.mkdir()
|
||||
(helper_dir / "issue1754_lock_probe.py").write_text(
|
||||
"import threading\n"
|
||||
"LOCK = threading.Lock()\n"
|
||||
"def try_acquire(result_queue):\n"
|
||||
" acquired = LOCK.acquire(timeout=1)\n"
|
||||
" if acquired:\n"
|
||||
" LOCK.release()\n"
|
||||
" result_queue.put(acquired)\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
sys.path.insert(0, str(helper_dir))
|
||||
try:
|
||||
import issue1754_lock_probe
|
||||
|
||||
issue1754_lock_probe.LOCK.acquire()
|
||||
try:
|
||||
# The held module-level lock models import/logging locks owned by a
|
||||
# sibling WebUI thread at the instant the manual cron worker starts.
|
||||
# fork clones the locked primitive into the child with no owner left
|
||||
# to release it; spawn re-imports a fresh module and can proceed.
|
||||
fork_queue = multiprocessing.get_context("fork").Queue()
|
||||
fork_exitcode, fork_acquired = _run_lock_probe_with_context(
|
||||
"fork", issue1754_lock_probe.try_acquire, fork_queue
|
||||
)
|
||||
spawn_queue = multiprocessing.get_context("spawn").Queue()
|
||||
spawn_exitcode, spawn_acquired = _run_lock_probe_with_context(
|
||||
"spawn", issue1754_lock_probe.try_acquire, spawn_queue
|
||||
)
|
||||
finally:
|
||||
issue1754_lock_probe.LOCK.release()
|
||||
for q in (locals().get("fork_queue"), locals().get("spawn_queue")):
|
||||
if q is not None:
|
||||
q.close()
|
||||
q.join_thread()
|
||||
finally:
|
||||
sys.modules.pop("issue1754_lock_probe", None)
|
||||
try:
|
||||
sys.path.remove(str(helper_dir))
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
assert fork_exitcode == 0
|
||||
assert fork_acquired is False
|
||||
assert spawn_exitcode == 0
|
||||
assert spawn_acquired is True
|
||||
|
||||
|
||||
def test_manual_cron_subprocess_drains_large_result_before_join(tmp_path):
|
||||
"""A >100 KB result must not deadlock the parent before it can persist output."""
|
||||
fake_pkg_root = tmp_path / "fake-cron-pkg"
|
||||
_write_fake_large_payload_cron_package(fake_pkg_root)
|
||||
|
||||
if _real_hermes_agent_editable_install_present():
|
||||
import pytest as _pytest
|
||||
_pytest.skip(
|
||||
"skipped on dev machines with an editable hermes-agent install — "
|
||||
"the spawn child resolves the real cron.scheduler first instead of "
|
||||
"the fake one written under HERMES_WEBUI_AGENT_DIR. Runs cleanly on CI."
|
||||
)
|
||||
# Use fork only for the outer test harness so this pytest module does not
|
||||
# need to be importable as a package. The product helper under test owns its
|
||||
# own multiprocessing context.
|
||||
@@ -86,7 +260,7 @@ def test_manual_cron_subprocess_drains_large_result_before_join(tmp_path):
|
||||
result_queue = ctx.Queue()
|
||||
runner = ctx.Process(
|
||||
target=_large_cron_payload_runner,
|
||||
args=(fake_pkg_root, tmp_path / "exec-profile", result_queue),
|
||||
args=(tmp_path / "exec-profile", result_queue),
|
||||
)
|
||||
runner.start()
|
||||
runner.join(10)
|
||||
@@ -105,7 +279,12 @@ def test_manual_cron_subprocess_drains_large_result_before_join(tmp_path):
|
||||
finally:
|
||||
result_queue.close()
|
||||
result_queue.join_thread()
|
||||
assert result == ("ok", True, 200_000, 200_000, None)
|
||||
tag, success, output_len, final_response_len, error = result
|
||||
assert tag == "ok"
|
||||
assert success is True
|
||||
assert output_len == 200_000
|
||||
assert final_response_len == 200_000
|
||||
assert error is None
|
||||
|
||||
|
||||
def test_manual_cron_run_does_not_hold_profile_lock_for_job_duration(tmp_path, monkeypatch):
|
||||
@@ -171,23 +350,33 @@ def test_manual_cron_run_does_not_hold_profile_lock_for_job_duration(tmp_path, m
|
||||
|
||||
|
||||
def test_cron_job_subprocess_executes_under_selected_profile_home(tmp_path, monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
def fake_run_job(job):
|
||||
import cron.scheduler as scheduler
|
||||
|
||||
return True, str(scheduler._hermes_home), "final", None
|
||||
|
||||
events = []
|
||||
_, cron_scheduler = _install_fake_cron(monkeypatch, fake_run_job, events)
|
||||
if _real_hermes_agent_editable_install_present():
|
||||
import pytest as _pytest
|
||||
_pytest.skip(
|
||||
"skipped on dev machines with an editable hermes-agent install — "
|
||||
"the spawn child resolves the real cron.scheduler first instead of "
|
||||
"the fake one written under HERMES_WEBUI_AGENT_DIR. Runs cleanly on CI."
|
||||
)
|
||||
exec_home = tmp_path / "exec-profile"
|
||||
|
||||
success, output, final_response, error = routes._run_cron_job_in_profile_subprocess(
|
||||
{"id": "job1574"}, exec_home
|
||||
ctx = multiprocessing.get_context("fork")
|
||||
result_queue = ctx.Queue()
|
||||
runner = ctx.Process(
|
||||
target=_selected_profile_home_runner,
|
||||
args=(exec_home, result_queue),
|
||||
)
|
||||
runner.start()
|
||||
runner.join(10)
|
||||
if runner.is_alive():
|
||||
runner.terminate()
|
||||
runner.join(5)
|
||||
result_queue.close()
|
||||
result_queue.join_thread()
|
||||
raise AssertionError("manual cron subprocess did not finish selected-profile probe")
|
||||
|
||||
assert success is True
|
||||
assert output == str(exec_home)
|
||||
assert final_response == "final"
|
||||
assert error is None
|
||||
assert cron_scheduler._hermes_home == Path("/tmp/hermes")
|
||||
try:
|
||||
result = result_queue.get(timeout=2)
|
||||
finally:
|
||||
result_queue.close()
|
||||
result_queue.join_thread()
|
||||
|
||||
assert result == ("ok", True, str(exec_home), "final", None)
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
from api import streaming
|
||||
|
||||
|
||||
CODEX_PLAN_LIMIT_ERROR = (
|
||||
"HTTP 429: {\"error\": {\"type\": \"usage_limit_exceeded\", "
|
||||
"\"message\": \"Plan limit reached. You've reached the limit of messages per 5 hours.\"}}"
|
||||
)
|
||||
|
||||
|
||||
def test_codex_oauth_usage_exhaustion_is_classified_as_quota():
|
||||
for err in [
|
||||
'Plan limit reached',
|
||||
'usage_limit_exceeded',
|
||||
'usage limit exceeded',
|
||||
"You've reached the limit of messages per 5 hours",
|
||||
"You've used up your usage",
|
||||
CODEX_PLAN_LIMIT_ERROR,
|
||||
]:
|
||||
classified = streaming._classify_provider_error(err, Exception(err))
|
||||
assert classified['type'] == 'quota_exhausted', err
|
||||
assert classified['label'] == 'Out of credits'
|
||||
assert 'credits' in classified['hint'].lower() or 'usage' in classified['hint'].lower()
|
||||
|
||||
|
||||
def test_silent_provider_failure_gets_specific_catch_all_error():
|
||||
classified = streaming._classify_provider_error('', None, silent_failure=True)
|
||||
|
||||
assert classified['type'] == 'no_response'
|
||||
assert classified['label'] == 'No response from provider'
|
||||
assert 'returned no content and no error' in classified['hint']
|
||||
|
||||
|
||||
def test_provider_error_payload_includes_bounded_redacted_details(monkeypatch):
|
||||
secret = 'sk-proj-' + ('a' * 80)
|
||||
raw_error = CODEX_PLAN_LIMIT_ERROR + ' token=' + secret
|
||||
|
||||
monkeypatch.setattr(streaming, '_redact_text', lambda text: text.replace(secret, '[REDACTED]'))
|
||||
payload = streaming._provider_error_payload(raw_error, 'quota_exhausted', 'Switch providers')
|
||||
|
||||
assert payload['message']
|
||||
assert secret not in payload['message']
|
||||
assert payload['details']
|
||||
assert secret not in payload['details']
|
||||
assert '[REDACTED]' in payload['details']
|
||||
assert len(payload['details']) <= 1200
|
||||
|
||||
|
||||
def test_frontend_renders_apperror_details_in_collapsible_block():
|
||||
messages_js = (streaming.Path(__file__).resolve().parent.parent / 'static' / 'messages.js').read_text()
|
||||
ui_js = (streaming.Path(__file__).resolve().parent.parent / 'static' / 'ui.js').read_text()
|
||||
style_css = (streaming.Path(__file__).resolve().parent.parent / 'static' / 'style.css').read_text()
|
||||
apperror_idx = messages_js.find("source.addEventListener('apperror'")
|
||||
warning_idx = messages_js.find("source.addEventListener('warning'", apperror_idx)
|
||||
assert apperror_idx != -1 and warning_idx != -1
|
||||
apperror_block = messages_js[apperror_idx:warning_idx]
|
||||
|
||||
assert 'd.details' in apperror_block
|
||||
assert 'provider_details:details' in apperror_block
|
||||
assert 'm.provider_details' in ui_js
|
||||
assert '<details class="provider-error-details"' in ui_js
|
||||
assert 'Provider details' in ui_js
|
||||
assert '.provider-error-details' in style_css
|
||||
@@ -0,0 +1,115 @@
|
||||
"""
|
||||
Regression tests for resolve_model_provider — issue #1744.
|
||||
|
||||
When an OpenRouter model ID ends in a colon-suffixed tag like ``:free``,
|
||||
``:beta``, ``:thinking``, the ``@provider:model`` qualifier produced by
|
||||
``model_with_provider_context`` collides with the ``rsplit(":", 1)`` grammar
|
||||
inside ``resolve_model_provider``. The resolver would incorrectly peel the
|
||||
suffix into the provider field instead of keeping it attached to the model.
|
||||
|
||||
E.g. ``@openrouter:tencent/hy3-preview:free`` was resolved as
|
||||
``model="free", provider="openrouter:tencent/hy3-preview"`` instead of the
|
||||
correct ``model="tencent/hy3-preview:free", provider="openrouter"``.
|
||||
|
||||
The fix (api/config.py ~line 1370) validates the rsplit result: if the
|
||||
provider hint is not a known provider and not a custom provider, it falls
|
||||
back to ``split(":", 1)`` so trailing suffixes stay with the model.
|
||||
"""
|
||||
|
||||
from api.config import resolve_model_provider, model_with_provider_context
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helper: simulate a config where provider != openrouter so that
|
||||
# model_with_provider_context actually qualifies the ID.
|
||||
# ---------------------------------------------------------------------------
|
||||
def _set_config_provider(provider: str, default_model: str = "claude-sonnet-4.6"):
|
||||
"""Temporarily set the model config provider for testing."""
|
||||
import api.config as cfg_mod
|
||||
old = dict(cfg_mod.cfg.get("model", {}))
|
||||
cfg_mod.cfg["model"] = {"provider": provider, "default": default_model}
|
||||
return old, cfg_mod
|
||||
|
||||
|
||||
def _restore_config(old, cfg_mod):
|
||||
cfg_mod.cfg["model"] = old
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_openrouter_free_suffix_survives_provider_qualification():
|
||||
"""tencent/hy3-preview:free must resolve correctly when qualified."""
|
||||
import api.config as cfg_mod
|
||||
old, cfg_mod = _set_config_provider("anthropic")
|
||||
try:
|
||||
qualified = model_with_provider_context("tencent/hy3-preview:free", "openrouter")
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "openrouter", f"expected provider='openrouter', got '{provider}'"
|
||||
assert model == "tencent/hy3-preview:free", f"expected model='tencent/hy3-preview:free', got '{model}'"
|
||||
finally:
|
||||
_restore_config(old, cfg_mod)
|
||||
|
||||
|
||||
def test_openrouter_free_suffix_nvidia():
|
||||
"""nvidia/nemotron-3-super-120b-a12b:free — same bug class."""
|
||||
import api.config as cfg_mod
|
||||
old, cfg_mod = _set_config_provider("anthropic")
|
||||
try:
|
||||
qualified = model_with_provider_context("nvidia/nemotron-3-super-120b-a12b:free", "openrouter")
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "openrouter"
|
||||
assert model == "nvidia/nemotron-3-super-120b-a12b:free"
|
||||
finally:
|
||||
_restore_config(old, cfg_mod)
|
||||
|
||||
|
||||
def test_openrouter_free_suffix_arcee():
|
||||
"""arcee-ai/trinity-large-preview:free — same bug class."""
|
||||
import api.config as cfg_mod
|
||||
old, cfg_mod = _set_config_provider("anthropic")
|
||||
try:
|
||||
qualified = model_with_provider_context("arcee-ai/trinity-large-preview:free", "openrouter")
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "openrouter"
|
||||
assert model == "arcee-ai/trinity-large-preview:free"
|
||||
finally:
|
||||
_restore_config(old, cfg_mod)
|
||||
|
||||
|
||||
def test_openrouter_thinking_suffix():
|
||||
"""Models ending in :thinking should also be preserved."""
|
||||
import api.config as cfg_mod
|
||||
old, cfg_mod = _set_config_provider("anthropic")
|
||||
try:
|
||||
qualified = model_with_provider_context("some/model:thinking", "openrouter")
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "openrouter"
|
||||
assert model == "some/model:thinking"
|
||||
finally:
|
||||
_restore_config(old, cfg_mod)
|
||||
|
||||
|
||||
def test_custom_provider_rsplit_still_works():
|
||||
"""custom:my-key:model must still parse correctly via rsplit."""
|
||||
qualified = "@custom:my-key:some-model"
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "custom:my-key", f"expected provider='custom:my-key', got '{provider}'"
|
||||
assert model == "some-model", f"expected model='some-model', got '{model}'"
|
||||
|
||||
|
||||
def test_known_provider_single_colon():
|
||||
"""@openrouter:simple-model — no suffix, should still work."""
|
||||
qualified = "@openrouter:simple-model"
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "openrouter"
|
||||
assert model == "simple-model"
|
||||
|
||||
|
||||
def test_known_provider_anthropic():
|
||||
"""@anthropic:claude-sonnet-4.6 — standard case."""
|
||||
qualified = "@anthropic:claude-sonnet-4.6"
|
||||
model, provider, _ = resolve_model_provider(qualified)
|
||||
assert provider == "anthropic"
|
||||
assert model == "claude-sonnet-4.6"
|
||||
Reference in New Issue
Block a user