diff --git a/docs/profile-isolation/README.md b/docs/profile-isolation/README.md index 206022706..f76c49fe4 100644 --- a/docs/profile-isolation/README.md +++ b/docs/profile-isolation/README.md @@ -6,8 +6,10 @@ Read order: 1. `technical-design.md` — architecture and precedence rules 2. `checklist.md` — implementation and acceptance checklist 3. `implementation-status.md` — what has already landed in code and what still needs review -4. `issue-body-templates.md` — GitHub issue body templates for this workstream -5. `README.md` — quick file map and why these files exist +4. `restart-recovery-runbook.md` — safe gateway/api_server/OpenWebUI restart and recovery workflow +5. `closing-notes.md` — merge/rollout summary for the latest hardening batch +6. `issue-body-templates.md` — GitHub issue body templates for this workstream +7. `README.md` — quick file map and why these files exist ## What belongs where diff --git a/docs/profile-isolation/closing-notes.md b/docs/profile-isolation/closing-notes.md new file mode 100644 index 000000000..b3e23105b --- /dev/null +++ b/docs/profile-isolation/closing-notes.md @@ -0,0 +1,119 @@ +# Profile Isolation / Download-Link Hardening — Closing Notes + +## Summary + +This batch closes the remaining follow-up items after principal-scoped session, memory, and managed-download isolation landed. + +The final scope completed here is: +- prevent live SSE streaming from briefly exposing host-local artifact paths such as `/Users/...` or `C:\...` +- carry `download_link_mode` one layer higher so OpenWebUI-facing runs can request private vs public managed-download behavior +- document a safe restart / recovery procedure for gateway + API server + OpenWebUI runs proxy + +## Landed repo changes + +### `gateway/platforms/api_server.py` +- added a streaming artifact rewrite guard for SSE output +- buffers split local-path prefixes until they can be rewritten safely +- rewrites completed local artifact references to managed-download links during streaming +- falls back to filename-only text if a buffered local path reaches stream end without a safe rewrite +- fixed the carry-buffer logic so normal assistant text is not delayed +- tightened path-prefix detection so `https://...` is not misclassified as a Windows-style drive path + +### `tests/gateway/test_api_server.py` +- updated the managed-download helper expectation to match the tuple return shape +- verified ordinary streamed text still appears immediately +- verified `None` tool-call sentinels do not break streaming +- verified `/v1/responses` streaming completed events carry rewritten managed-download URLs and hook metadata + +### `tests/gateway/test_openwebui_runs_proxy_handoff.py` +- added regression coverage for extracting `download_link_mode` +- added regression coverage ensuring owner metadata + link mode are passed into managed-download URL rewriting + +### `docs/profile-isolation/*` +- updated `README.md` read order +- updated `implementation-status.md` to reflect the streaming rewrite fix and OpenWebUI proxy hook landing +- added `restart-recovery-runbook.md` +- added this closing-notes file + +## Non-repo local changes + +These changes are operational/local and are not part of the git repo: + +### `/Users/hermes/.hermes/scripts/openwebui_runs_proxy.py` +- now accepts `download_link_mode` from top-level request payload or `metadata.hermes.download_link_mode` +- forwards `download_link_mode` upstream to the Hermes API server +- rewrites local file paths using owner-aware managed-download mode instead of always minting public links +- appends a small note when private mode is used and the caller may want a future public link + +### `~/Library/LaunchAgents/ai.hermes.gateway.plist` +- removed `--replace` from the steady-state LaunchAgent command line +- this change only affects future reloads/restarts; it does not rewrite the currently running process until launchd reloads the job + +## Verification performed + +Verified successfully: + +```bash +python3 -m py_compile \ + gateway/platforms/api_server.py \ + tests/gateway/test_api_server.py \ + tests/gateway/test_session_api.py \ + tests/gateway/test_openwebui_runs_proxy_handoff.py \ + /Users/hermes/.hermes/scripts/openwebui_runs_proxy.py + +pytest -q \ + tests/gateway/test_api_server.py \ + tests/gateway/test_session_api.py \ + tests/gateway/test_openwebui_runs_proxy_handoff.py +``` + +Observed result: +- `221 passed, 5 warnings in 16.53s` + +Warnings were third-party deprecation warnings from `pkg_resources` under `lark_oapi`, not failures in the isolation changes. + +## Live runtime facts verified before restart + +Observed from the live system during this session: +- `8642/health` was healthy +- `8653/health` was healthy +- `8646/line/webhook/health` was healthy +- the active gateway LaunchAgent runtime still showed `gateway run --replace` before reload +- `launchctl print gui/$(id -u)/ai.hermes.gateway` showed repeated restarts (`runs = 41`, `forks = 2400`, `last terminating signal = Killed: 9`) + +Interpretation: +- current service was up +- but the old long-lived launchd configuration had real flap risk and should not remain on `--replace` + +## Restart recommendation + +Use the runbook in `restart-recovery-runbook.md`. + +Short version: +1. ensure tests are green +2. ensure `ai.hermes.gateway.plist` no longer contains `--replace` +3. reload gateway first and wait for `8642/health` +4. reload OpenWebUI runs proxy second and wait for `8653/health` +5. re-check `8646/line/webhook/health` +6. inspect `launchctl print` to confirm gateway live arguments no longer include `--replace` +7. smoke-test streaming + managed-download rewrite immediately after restart + +## Known limitation in this chat session + +Direct restart commands from this active agent session were blocked by safety guards because restarting the supervised gateway from inside the gateway tool context can create SIGTERM/self-respawn loops. + +That blocker is expected behavior, not a code failure. + +The correct way to perform the live restart is from an external shell / independent operator context. A background delegated verifier was dispatched to try the isolated-path verification separately. + +## Suggested PR / merge summary + +Title: +- `fix: harden streaming artifact rewrite and propagate download_link_mode to runs proxy` + +Body bullets: +- prevent live SSE deltas from leaking host-local artifact paths before managed-download rewrite +- preserve ordinary streamed text and tool-call progress semantics +- propagate `download_link_mode` through the OpenWebUI runs proxy +- add regression coverage for streaming rewrite behavior and proxy link-mode extraction +- document safe restart/recovery procedure and remove `--replace` from the steady-state gateway LaunchAgent config diff --git a/docs/profile-isolation/implementation-status.md b/docs/profile-isolation/implementation-status.md index 7297195fb..dddaae2e8 100644 --- a/docs/profile-isolation/implementation-status.md +++ b/docs/profile-isolation/implementation-status.md @@ -73,10 +73,17 @@ Implemented: - `api_server` request bodies now accept `download_link_mode` and return machine-readable `hermes.download_link_mode*` metadata so callers can prompt for private vs public link behavior - `/v1/responses` non-streaming payloads now keep `response.output[*]` aligned with the rewritten managed-download URL instead of exposing a host-local artifact path in the assistant message item - `/v1/responses` streaming terminal events now carry the same managed-download rewrite and `hermes.download_link_mode` metadata as the non-streaming path +- live streaming delta paths now hold back split local file-path prefixes and rewrite them before delivery instead of briefly exposing `/Users/...` or `C:\...` during SSE streaming +- the OpenWebUI runs proxy now accepts `download_link_mode`, forwards it upstream, and rewrites local artifact paths using owner-aware private/public managed-download selection instead of always minting public links Primary files: - `hermes_cli/managed_downloads.py` - `tests/hermes_cli/test_managed_downloads.py` +- `gateway/platforms/api_server.py` +- `tests/gateway/test_api_server.py` +- `tests/gateway/test_session_api.py` +- `/Users/hermes/.hermes/scripts/openwebui_runs_proxy.py` +- `tests/gateway/test_openwebui_runs_proxy_handoff.py` ## Verified tests already exercised for this workstream @@ -98,7 +105,13 @@ These are not known regressions, but they remain explicit review targets wheneve - gateway UX hooks that should ask the user whether a link should stay owner-bound or become public - any future response-chain storage outside `response_store.db` - non-API file-delivery paths that may need the same principal-aware mode selection contract -- live token-by-token streaming deltas may still expose a host-local artifact path before the terminal `response.completed` envelope is rewritten; this is a presentation-path follow-up, not a principal-ownership bypass + +## Operational finding from live runtime inspection + +- the current macOS LaunchAgent `~/Library/LaunchAgents/ai.hermes.gateway.plist` was found running `gateway run --replace` +- `launchctl print gui/$(id -u)/ai.hermes.gateway` showed repeated restarts (`runs=41`, `forks=2400`, last terminating signal `Killed: 9`) while the service was still healthy +- for a long-lived supervised LaunchAgent, `--replace` should be removed and reserved for explicit operator takeover flows instead of steady-state service startup +- see `restart-recovery-runbook.md` for the safe reload / health-check sequence before applying that launchd change ## Current invariant diff --git a/docs/profile-isolation/restart-recovery-runbook.md b/docs/profile-isolation/restart-recovery-runbook.md new file mode 100644 index 000000000..0f47486d0 --- /dev/null +++ b/docs/profile-isolation/restart-recovery-runbook.md @@ -0,0 +1,214 @@ +# Gateway / API Server / OpenWebUI Restart & Recovery Runbook + +## Purpose + +Use this runbook when code changes affecting `gateway`, `api_server`, managed downloads, or the OpenWebUI runs proxy need to go live safely. + +This document answers four operator questions: + +1. Do these changes need a restart? +2. How do we restart without causing restart loops or half-broken state? +3. What should we verify before and after restart? +4. How do we diagnose and recover when restart fails or keeps flapping? + +## Does this work require restart? + +Yes for the current workstream. + +These changes modify live Python code paths: +- `gateway/platforms/api_server.py` +- `hermes_cli/managed_downloads.py` +- `/Users/hermes/.hermes/scripts/openwebui_runs_proxy.py` + +If the gateway and/or the OpenWebUI runs proxy are already running, their existing Python processes will keep serving the old code until they are restarted. + +## Live findings from this session + +Verified during runtime inspection: +- `http://127.0.0.1:8642/health` returned `200` +- `http://127.0.0.1:8653/health` returned `200` +- `http://127.0.0.1:8646/line/webhook/health` returned `200` +- `launchctl print gui/$(id -u)/ai.hermes.gateway` showed the active LaunchAgent still running: + - `gateway run --replace` + - `runs = 41` + - `forks = 2400` + - `last terminating signal = Killed: 9` + +Interpretation: +- the service is currently healthy +- but the long-lived LaunchAgent configuration is risky because `--replace` can create self-replacement churn in a supervised service +- this should be corrected before or as part of the next controlled reload + +## Safe rollout sequence + +### Phase A — preflight (do not restart yet) + +1. Confirm modified code compiles. +2. Run targeted tests for: + - `tests/gateway/test_api_server.py` + - `tests/gateway/test_session_api.py` + - `tests/gateway/test_openwebui_runs_proxy_handoff.py` +3. Confirm current health endpoints before touching launchd: + - `8642/health` + - `8653/health` + - `8646/line/webhook/health` +4. Record current listeners / PIDs: + - port `8642` + - port `8653` +5. Confirm whether the gateway LaunchAgent still contains `--replace`. + +Completion criteria: +- tests pass +- current runtime is healthy +- we know whether launchd config itself needs correction + +### Phase B — fix the launchd steady-state configuration + +For a long-lived LaunchAgent, prefer: +- `gateway run` + +Avoid in steady-state launchd config: +- `gateway run --replace` + +Reason: +- `--replace` is appropriate for explicit operator takeover flows +- but in a persistent `RunAtLoad + KeepAlive` LaunchAgent it increases the risk of self-replacement races, SIGTERM churn, and restart flapping + +### Phase C — reload in a controlled order + +1. Update the LaunchAgent plist if it still contains `--replace`. +2. Unload / bootstrap or kickstart the gateway service cleanly. +3. Verify the new gateway process command line no longer includes `--replace`. +4. Re-check `8642/health` before treating OpenWebUI as recovered. +5. Re-check `8653/health` and run a real OpenWebUI proxy smoke request. + +Completion criteria: +- gateway healthy on `8642` +- runs proxy healthy on `8653` +- live command line matches expected startup arguments + +## Post-restart verification checklist + +### Core health +- [ ] `8642/health` returns `200` +- [ ] `8653/health` returns `200` +- [ ] `8646/line/webhook/health` returns `200` +- [ ] `lsof` shows one expected listener on `8642` +- [ ] `lsof` shows one expected listener on `8653` + +### Launchd / process state +- [ ] `launchctl print gui/$(id -u)/ai.hermes.gateway` shows `gateway run` without `--replace` +- [ ] PID changed only once during the controlled restart +- [ ] no immediate follow-up SIGTERM / Killed:9 churn appears in logs + +### Functional checks +- [ ] non-streaming `/v1/responses` still rewrites local artifact paths to managed download URLs +- [ ] streaming `/v1/responses` no longer leaks `/Users/...` or `C:\...` in live deltas +- [ ] `/v1/chat/completions` streaming still emits normal assistant text and keepalives +- [ ] OpenWebUI proxy rewrites local file paths to clickable managed download links +- [ ] `download_link_mode=private/public` still propagates correctly + +## Failure modes and how to diagnose them + +### 1. Service does not come back at all + +Symptoms: +- `8642/health` fails +- no listener on `8642` +- launchd job exists but no stable PID + +Check first: +- `gateway.error.log` +- `gateway.log` +- `launchctl print gui/$(id -u)/ai.hermes.gateway` +- command line / working directory / venv path in the plist + +Most likely causes: +- syntax/import error in changed Python code +- bad venv path or stale interpreter path +- environment mismatch +- port bind failure + +### 2. Service comes back, then keeps restarting + +Symptoms: +- `runs` / `forks` climb quickly in `launchctl print` +- repeated `Received SIGTERM` +- `last terminating signal = Killed: 9` or similar + +Check first: +- whether the LaunchAgent still contains `--replace` +- whether another helper job is repeatedly kickstarting the service +- whether a watchdog or restart helper is racing the main LaunchAgent + +Most likely causes: +- supervised launchd service using `--replace` +- duplicate restart authority (helper + launchd + watchdog) +- repeated crash on startup with KeepAlive immediately reviving it + +### 3. Gateway is healthy but OpenWebUI still broken + +Symptoms: +- `8642/health` is healthy +- `8653` is unhealthy or returns bad stream behavior + +Check first: +- runs proxy process / listener on `8653` +- runs proxy logs +- whether the proxy is still serving old code +- whether it can reach upstream `8642` + +Most likely causes: +- proxy not restarted after code change +- upstream base URL mismatch +- missing dependency / import in the proxy process +- old proxy process still running + +### 4. Health endpoints are fine but behavior is stale + +Symptoms: +- routes work, but old behavior remains +- new download-link mode / streaming rewrite behavior not visible + +Check first: +- live PID start times +- whether the file on disk matches the process that launchd is running +- whether only one process instance exists per port + +Most likely causes: +- service not actually restarted +- edited one file, but launchd runs another path +- stale long-lived proxy/gateway process + +## Recommended diagnostic order when restart fails + +1. Confirm health endpoints and listeners. +2. Inspect launchd runtime state (`launchctl print`). +3. Inspect recent gateway stdout/stderr logs with timestamps around the restart. +4. Verify the exact source file on disk contains the expected fix. +5. If needed, stop auto-restart pressure and run the gateway/proxy once in foreground with the same venv and env vars. +6. Only after the foreground process is healthy, restore launchd supervision. + +## Anti-flap rules + +- Do not combine multiple independent restart authorities unless one is clearly primary. +- Do not leave `--replace` inside a steady-state launchd job. +- Do not restart blindly when health endpoints are already good; first confirm whether the issue is stale behavior vs runtime outage. +- Do not declare success from static code inspection alone; always verify the live listener and health endpoint. + +## Rollback rule + +If restart introduces instability: +1. stop the restart loop source +2. revert to the last known-good commit / script version +3. start the service once in foreground or via a controlled launchd reload +4. verify health and behavior before restoring automatic supervision + +## Current recommendation for this environment + +Before the next production restart of the gateway: +- remove `--replace` from `~/Library/LaunchAgents/ai.hermes.gateway.plist` +- keep the gateway in `RunAtLoad + KeepAlive`, but with plain `gateway run` +- restart in a controlled sequence +- verify both `8642` and `8653` +- smoke-test streaming and managed-download rewriting immediately after restart diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 76bb474c7..4f91c0485 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -717,6 +717,114 @@ _LOCAL_PATH_LINE_RE = re.compile( r"(?P(?=$|[\s)\]}'\",;:]))", re.MULTILINE, ) +_STREAMING_LOCAL_PATH_PREFIX_RE = re.compile(r"(?:(?<=^)|(?<=[\s(\[{'\":]))(?:file://|/Users/|[A-Za-z]:[/\\])") +_STREAMING_LOCAL_PATH_TOKEN_RE = re.compile( + r"(?P(?:file://(?:/[^^\n\s`\"'<>]+(?:[^\S\n]+[^\s`\"'<>]+)*)|(?:/Users/|[A-Za-z]:[/\\])[^\n\r]*?\.(?:pptx|pdf|docx|xlsx|csv|tsv|txt|md|zip|json|html|png|jpg|jpeg|webp|gif|mp4|mov|mp3|wav|ogg)))" + r"(?P[)\],。!?,.!?\s]|$)", + re.IGNORECASE, +) +_STREAMING_LOCAL_PATH_PREFIXES = ("file://", "/Users/") + + +def _streaming_local_path_carry_len(text: str) -> int: + """Return how many trailing chars to keep for a possible split path prefix.""" + if not text: + return 0 + carry = 0 + for prefix in _STREAMING_LOCAL_PATH_PREFIXES: + max_len = min(len(prefix) - 1, len(text)) + for n in range(1, max_len + 1): + if text.endswith(prefix[:n]): + carry = max(carry, n) + # Windows path starts like ``C:``, ``C:/`` or ``C:\``; keep the trailing + # fragment only when the *entire trailing buffer* is just that short prefix. + if re.fullmatch(r"[A-Za-z]", text): + carry = max(carry, 1) + if re.fullmatch(r"[A-Za-z]:", text): + carry = max(carry, 2) + if re.fullmatch(r"[A-Za-z]:[\\/]", text): + carry = max(carry, 3) + return carry + + +def _replace_local_artifact_paths_with_filenames(text: str) -> str: + def _path_repl(match: "re.Match[str]") -> str: + raw_path = str(match.group("path") or "") + prefix = str(match.groupdict().get("prefix") or "") + suffix = str(match.groupdict().get("suffix") or "") + normalized = raw_path.replace("file://", "", 1).replace("\\", "/").rstrip("`\"',.;:)}]。!?,.!?") + filename = Path(normalized).name or raw_path + return f"{prefix}{filename}{suffix}" + + out = _LOCAL_FILE_URI_RE.sub(lambda m: Path(str(m.group("path") or "").replace('\\', '/')).name or m.group(0), text) + return _LOCAL_PATH_LINE_RE.sub(_path_repl, out) + + +class _StreamingArtifactRewriteGuard: + """Hold back host-local artifact paths until they can be rewritten safely. + + Streaming deltas can split an absolute file path across several chunks. If we + forward those chunks immediately, remote clients briefly see `/Users/...` + before the terminal payload rewrites the final answer to a managed download + URL. This guard keeps a short carry buffer for possible path-prefix splits, + rewrites complete local-path tokens to managed download URLs when possible, + and falls back to filename-only text if a buffered path reaches stream end + without becoming safely rewritable. + """ + + def __init__(self, *, owner_metadata: Optional[Dict[str, Any]] = None, link_mode: Optional[str] = None) -> None: + self._buffer = "" + self._owner_metadata = owner_metadata + self._link_mode = link_mode + + def feed(self, delta: str) -> List[str]: + if delta: + self._buffer += str(delta) + return self._drain(final=False) + + def flush(self) -> List[str]: + return self._drain(final=True) + + def _drain(self, *, final: bool) -> List[str]: + emitted: List[str] = [] + while self._buffer: + match = _STREAMING_LOCAL_PATH_PREFIX_RE.search(self._buffer) + if match is None: + if final: + emitted.append(self._buffer) + self._buffer = "" + else: + keep = _streaming_local_path_carry_len(self._buffer) + emit_len = len(self._buffer) - keep + if emit_len > 0: + emitted.append(self._buffer[:emit_len]) + self._buffer = self._buffer[emit_len:] + break + + if match.start() > 0: + emitted.append(self._buffer[: match.start()]) + self._buffer = self._buffer[match.start():] + + token_match = _STREAMING_LOCAL_PATH_TOKEN_RE.match(self._buffer) + if token_match is None: + if final: + emitted.append(_replace_local_artifact_paths_with_filenames(self._buffer)) + self._buffer = "" + break + + raw_token = str(token_match.group("path") or "") + trailer = str(token_match.group("trailer") or "") + rewritten, rewrite_info = _rewrite_local_artifact_references_for_remote_client( + raw_token, + owner_metadata=self._owner_metadata, + link_mode=self._link_mode, + ) + if int(rewrite_info.get("rewritten_count", 0) or 0) <= 0: + rewritten = _replace_local_artifact_paths_with_filenames(raw_token) + emitted.append(f"{rewritten}{trailer}") + self._buffer = self._buffer[token_match.end():] + + return [chunk for chunk in emitted if chunk] def _resolve_media_to_data_urls(text: str) -> str: @@ -2939,6 +3047,10 @@ class APIServerAdapter(BasePlatformAdapter): message_id = f"msg_{uuid.uuid4().hex}" run_id = f"run_{uuid.uuid4().hex}" seq = 0 + stream_guard = _StreamingArtifactRewriteGuard( + owner_metadata=self._managed_download_owner_metadata(identity_context), + link_mode=requested_download_link_mode, + ) def _event_payload(name: str, payload: Dict[str, Any]) -> tuple[str, Dict[str, Any]]: nonlocal seq @@ -2965,7 +3077,8 @@ class APIServerAdapter(BasePlatformAdapter): def _delta(delta: str) -> None: if delta: - _enqueue("assistant.delta", {"message_id": message_id, "delta": delta}) + for safe_delta in stream_guard.feed(delta): + _enqueue("assistant.delta", {"message_id": message_id, "delta": safe_delta}) def _tool_progress(event_type: str, tool_name: str = None, preview: str = None, args=None, **kwargs) -> None: if event_type == "reasoning.available": @@ -2995,6 +3108,8 @@ class APIServerAdapter(BasePlatformAdapter): owner_metadata=self._managed_download_owner_metadata(identity_context), link_mode=requested_download_link_mode, ) + for safe_delta in stream_guard.flush(): + _enqueue("assistant.delta", {"message_id": message_id, "delta": safe_delta}) hermes_meta = _download_link_hook_metadata(download_link_info, requested_mode=requested_download_link_mode) effective_session_id = result.get("session_id", session_id) if isinstance(result, dict) else session_id turn_messages = self._turn_transcript_messages(history, user_message, result) if isinstance(result, dict) else [] @@ -3449,6 +3564,7 @@ class APIServerAdapter(BasePlatformAdapter): sse_headers["X-Hermes-Session-Key"] = gateway_session_key response = web.StreamResponse(status=200, headers=sse_headers) await response.prepare(request) + stream_guard = _StreamingArtifactRewriteGuard() try: last_activity = time.monotonic() @@ -3479,12 +3595,13 @@ class APIServerAdapter(BasePlatformAdapter): f"event: hermes.tool.progress\ndata: {event_data}\n\n".encode() ) else: - content_chunk = { - "id": completion_id, "object": "chat.completion.chunk", - "created": created, "model": model, - "choices": [{"index": 0, "delta": {"content": item}, "finish_reason": None}], - } - await response.write(f"data: {json.dumps(content_chunk)}\n\n".encode()) + for safe_item in stream_guard.feed(str(item)): + content_chunk = { + "id": completion_id, "object": "chat.completion.chunk", + "created": created, "model": model, + "choices": [{"index": 0, "delta": {"content": safe_item}, "finish_reason": None}], + } + await response.write(f"data: {json.dumps(content_chunk)}\n\n".encode()) return time.monotonic() # Stream content chunks as they arrive from the agent @@ -3514,6 +3631,16 @@ class APIServerAdapter(BasePlatformAdapter): last_activity = await _emit(delta) + # Flush any buffered tail that was held back to avoid leaking a + # split local artifact path across chunks. + for safe_tail in stream_guard.flush(): + tail_chunk = { + "id": completion_id, "object": "chat.completion.chunk", + "created": created, "model": model, + "choices": [{"index": 0, "delta": {"content": safe_tail}, "finish_reason": None}], + } + await response.write(f"data: {json.dumps(tail_chunk)}\n\n".encode()) + # Get usage from completed agent. The agent can fail two ways # after the content queue terminates cleanly: (1) ``agent_task`` # raises, or (2) it returns a ``result`` dict flagged @@ -3680,6 +3807,11 @@ class APIServerAdapter(BasePlatformAdapter): response = web.StreamResponse(status=200, headers=sse_headers) await response.prepare(request) request_profile = self._effective_identity_profile(identity_context) + owner_metadata = self._managed_download_owner_metadata(identity_context) + stream_guard = _StreamingArtifactRewriteGuard( + owner_metadata=owner_metadata, + link_mode=requested_download_link_mode, + ) with self._profile_scope(request_profile): response_store = self._ensure_response_store() @@ -3818,15 +3950,16 @@ class APIServerAdapter(BasePlatformAdapter): async def _emit_text_delta(delta_text: str) -> None: await _open_message_item() - final_text_parts.append(delta_text) - await _write_event("response.output_text.delta", { - "type": "response.output_text.delta", - "item_id": message_item_id, - "output_index": message_output_index, - "content_index": 0, - "delta": delta_text, - "logprobs": [], - }) + for safe_delta_text in stream_guard.feed(delta_text): + final_text_parts.append(safe_delta_text) + await _write_event("response.output_text.delta", { + "type": "response.output_text.delta", + "item_id": message_item_id, + "output_index": message_output_index, + "content_index": 0, + "delta": safe_delta_text, + "logprobs": [], + }) async def _emit_tool_started(payload: Dict[str, Any]) -> str: """Emit response.output_item.added for a function_call. @@ -4024,6 +4157,16 @@ class APIServerAdapter(BasePlatformAdapter): # Flush any final batched text before processing result if _batch_buf: await _flush_batch() + for safe_tail in stream_guard.flush(): + final_text_parts.append(safe_tail) + await _write_event("response.output_text.delta", { + "type": "response.output_text.delta", + "item_id": message_item_id, + "output_index": message_output_index, + "content_index": 0, + "delta": safe_tail, + "logprobs": [], + }) # Pick up agent result + usage from the completed task try: diff --git a/tests/gateway/test_api_server.py b/tests/gateway/test_api_server.py index c0aff104d..56e7f9e78 100644 --- a/tests/gateway/test_api_server.py +++ b/tests/gateway/test_api_server.py @@ -95,13 +95,14 @@ class TestApiServerManagedDownloads: artifact = home / "internal-assistant-intro.html" artifact.write_text("ok", encoding="utf-8") - rewritten = _rewrite_local_artifact_references_for_remote_client( + rewritten, meta = _rewrite_local_artifact_references_for_remote_client( f"HTML 版:\nfile://{artifact}\n純路徑:\n{artifact}\nMEDIA:{artifact}" ) assert "file://" not in rewritten assert str(artifact) not in rewritten assert rewritten.count("https://hermesadmin.bremen.com.tw/downloads/s-") == 3 + assert meta["rewritten_count"] == 3 @pytest.mark.asyncio async def test_chat_completions_returns_managed_download_url_for_local_artifact(self, adapter, _isolate_hermes_home): diff --git a/tests/gateway/test_openwebui_runs_proxy_handoff.py b/tests/gateway/test_openwebui_runs_proxy_handoff.py index cee6362b0..1676eb06f 100644 --- a/tests/gateway/test_openwebui_runs_proxy_handoff.py +++ b/tests/gateway/test_openwebui_runs_proxy_handoff.py @@ -47,3 +47,32 @@ def test_upstream_identity_headers_include_standardized_identity_fields(): assert headers["X-Hermes-User-Id"] == "openwebui-user-42" assert headers["X-Hermes-User-Email"] == "vivian@bremen.com.tw" assert headers["X-Hermes-User-Name"] == "Vivian" + + +def test_extract_download_link_mode_accepts_top_level_and_metadata_hermes(): + proxy = _load_proxy_module() + assert proxy._extract_download_link_mode({"download_link_mode": "public"}) == "public" + assert proxy._extract_download_link_mode({"metadata": {"hermes": {"download_link_mode": "private"}}}) == "private" + + +def test_rewrite_local_paths_to_download_links_passes_owner_metadata_and_link_mode(monkeypatch): + proxy = _load_proxy_module() + seen = {} + + def _fake_build_public_download_url(path, *, owner_metadata=None, link_mode=None): + seen["path"] = path + seen["owner_metadata"] = owner_metadata + seen["link_mode"] = link_mode + return "https://hermesadmin.bremen.com.tw/downloads/s-demo" + + monkeypatch.setattr(proxy, "build_public_download_url", _fake_build_public_download_url) + rewritten, links = proxy._rewrite_local_paths_to_download_links( + "/Users/hermes/result.pdf", + owner_metadata={"email": "vivian@bremen.com.tw", "external_user_id": "openwebui-user-42"}, + link_mode="private", + ) + assert rewritten == "[result.pdf](https://hermesadmin.bremen.com.tw/downloads/s-demo)" + assert links == [("result.pdf", "https://hermesadmin.bremen.com.tw/downloads/s-demo")] + assert seen["path"] == "/Users/hermes/result.pdf" + assert seen["owner_metadata"]["email"] == "vivian@bremen.com.tw" + assert seen["link_mode"] == "private"