Skip to content

feat(chat): add WebUI prefill script hook#2888

Merged
4 commits merged into
nesquena:masterfrom
AJV20:feat/webui-prefill-script
May 27, 2026
Merged

feat(chat): add WebUI prefill script hook#2888
4 commits merged into
nesquena:masterfrom
AJV20:feat/webui-prefill-script

Conversation

@AJV20
Copy link
Copy Markdown
Contributor

@AJV20 AJV20 commented May 25, 2026

Thinking Path

  • WebUI already supports file-backed prefill_messages_file context for browser-originated turns.
  • Some Hermes deployments generate recall context dynamically from local notes systems such as Joplin, Obsidian, Notion, or llm-wiki.
  • Requiring an external cron/cache wrapper makes WebUI less direct than other Hermes surfaces for those deployments.
  • This PR adds a WebUI-specific, explicit opt-in script hook that normalizes script output into ephemeral prefill messages while keeping message bodies out of browser-visible status.

What Changed

  • Adds webui_prefill_messages_script and HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT support in api/streaming.py.
  • Keeps legacy prefill_messages_script ignored by WebUI unless the new WebUI-specific key is used.
  • Accepts script output as:
    • OpenAI-style JSON message list
    • JSON object with a messages list
    • plain text, wrapped as one system prefill message
  • Adds timeout handling via webui_prefill_messages_script_timeout / HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT_TIMEOUT with a 5s default and 30s cap.
  • Redacts script errors before surfacing compact status to the browser.
  • Documents the opt-in hook in README.md and ARCHITECTURE.md.
  • Adds a release-note-ready CHANGELOG.md entry.

Why It Matters

This lets users keep WebUI context parity with other Hermes surfaces when their environment uses a local recall script for third-party notes or knowledge sources. It stays provider-agnostic and default-off rather than hard-coding Joplin or any one notes system into WebUI.

Verification

  • python3.11 -m py_compile api/streaming.py
  • python3.11 -m pytest tests/test_webui_prefill_context.py -q -o addopts= — 8 passed
  • git diff --check

Risks / Follow-ups

  • This is an explicit local command execution hook, so it is disabled unless an administrator configures the WebUI-specific key or env var.
  • The browser status event intentionally reports only source/label/count/redacted errors and never exposes prefill message bodies.
  • Script output is still operator-controlled local context. Deployments should keep scripts small, deterministic, and free of secrets in stdout.

@AJV20
Copy link
Copy Markdown
Contributor Author

AJV20 commented May 25, 2026

Follow-up hardening pushed in befee0e0:

  • Added a focused timeout regression for the WebUI-specific prefill script hook.
  • Added a 256 KiB stdout cap before parsing script output, with a regression test for oversized output.
  • Updated README/CHANGELOG to document the output cap.

Design note: the legacy prefill_messages_script key is intentionally still ignored by WebUI unless the new WebUI-specific opt-in (webui_prefill_messages_script / HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT) is configured. That keeps existing CLI/Telegram/session-start recall scripts from being executed unexpectedly on browser-originated turns.

Verification:

  • python3.11 -m py_compile api/streaming.py
  • python3.11 -m pytest tests/test_webui_prefill_context.py -q -o addopts= → 10 passed
  • git diff --check

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Summary

Read the PR diff against origin/master and the new tests/test_webui_prefill_context.py. The shape is sound: a WebUI-only webui_prefill_messages_script / HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT opt-in, hard 30s timeout cap with 5s default, 256 KiB stdout cap, output redacted before the browser sees an error, and the public status helper strips message bodies. CI green on 3.11/3.12/3.13. The follow-up commit befee0e0 adds the timeout regression and oversized-stdout regression that I'd otherwise have asked for.

Code reference

api/streaming.py:346-380 (PR HEAD) gates the new script path entirely behind the WebUI-specific key:

def _load_prefill_messages_script(config_data: dict) -> dict:
    script_raw = os.getenv("HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT", "") or config_data.get("webui_prefill_messages_script")
    if not script_raw:
        return _prefill_not_configured()
    ...
    proc = subprocess.run(command, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
                          timeout=_prefill_script_timeout(config_data), check=False)

And _load_webui_prefill_context (api/streaming.py:382-400) tries script first, then falls back to prefill_messages_file, matching the README precedence note.

Diagnosis / review notes

A few things worth tightening before merge:

1. The "legacy prefill_messages_script is intentionally ignored" framing is misleading. I grepped ~/.hermes/hermes-agent for prefill_messages_script / PREFILL_MESSAGES_SCRIPT and there are zero matches — the agent only ships prefill_messages_file (e.g. hermes_cli/config.py:1250, cli.py:3016, gateway/run.py:2683). There's no legacy script key to be compatible with; the WebUI-namespaced key is the only one that exists anywhere. The PR body and code comments imply the opposite. Either drop the "legacy" framing in the docstring at api/streaming.py:382-388 and the follow-up comment, or coordinate with the agent so the naming converges before this lands.

2. Path-resolution inconsistency between string and list config shapes. _prefill_script_command (api/streaming.py:319-327) only resolves relative paths when the config is a single-element string:

parts = shlex.split(str(raw or ""))
if not parts: return []
if len(parts) == 1:
    parts[0] = str(_resolve_prefill_path(parts[0]))
return parts

So webui_prefill_messages_script: "./recall.py" resolves against the config dir, but webui_prefill_messages_script: ["./recall.py"] (the list shape your README example uses with ["python3", "/path/..."]) does not — Path.cwd() of the WebUI process will be used instead. That's a foot-gun for anyone who configures the list form with a relative argv[0]. Suggest applying _resolve_prefill_path to parts[0] unconditionally when the list arrives, or documenting that list shapes must use absolute paths.

3. Per-turn subprocess on the SSE hot path. This runs on every browser-originated turn before the SSE stream opens (api/streaming.py:4099-4106), holding a worker for up to webui_prefill_messages_script_timeout. With the 5s default that's a noticeable preamble for users with a misconfigured/slow script. Worth either (a) noting in the README that the script blocks the user's first token, or (b) wiring a fast-path cache keyed on session/recent-prompt so repeated turns don't reshell. Not a blocker — just call it out so operators know what they're opting into.

4. No environment scrub. subprocess.run inherits the parent env, including HERMES_* keys and any provider credentials in the WebUI's environment. The redactor only sweeps script output before surfacing errors; it doesn't protect the script input env. For a Joplin/Obsidian/Notion bridge this is usually fine, but it's worth a one-liner in the README noting the script inherits the WebUI process env and should be considered trusted local code.

Test plan

The new tests in tests/test_webui_prefill_context.py cover the right axes: JSON list, JSON-object-with-messages, plain text wrap, script-takes-precedence over file, timeout, oversized-stdout, redactor, and the public-status body strip. Reading the JSON-list test (test_webui_prefill_script_loads_json_messages) confirms the tool-role drop is exercised. The only gap I'd add is a test for the list-shape-with-relative-argv case once item #2 above is decided — right now the behaviour there is implicit.

Solid PR overall. None of the above are blockers; #1 (docstring framing) and #2 (path resolution) are worth a quick follow-up commit.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Thanks for the contribution @AJV20! Holding this PR for now while we work through a few concerns from the automated triage pass (see previous comment).

Specific things we'd like to see before merging:

  1. Legacy prefill claim — the PR description mentions "legacy prefill_messages_script" but a grep of ~/.hermes/hermes-agent shows no such config key exists. If this is meant to replace something, can you clarify what? If it's a new feature, the framing should be "add prefill script" not "make the prefill script a webui setting".

  2. Path resolution edge case_prefill_script_command only path-resolves the string config shape. The list form (e.g. ["./prefill.sh", "--arg"]) leaves argv[0] unresolved relative to the WebUI working directory, which means it won't run reliably from arbitrary launch contexts. A regression test for the list-shape-with-relative-path case would catch this.

  3. Streaming hot pathapi/streaming.py is a sensitive surface (every chat turn goes through it). Could you add a fast-path for when prefill_script_command is unset so the subprocess machinery doesn't fire on every browser turn?

Happy to chat through any of these in the comments if helpful. The general feature direction is fine — we just want the safety rails in before it lands on the streaming path.

@nesquena-hermes nesquena-hermes closed this pull request by merging all changes into nesquena:master in 08e9ce3 May 27, 2026
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Merged in Release DM / v0.51.141 (stage-batch23 — 4-PR second hold-bucket pass with PRs #2506 #2792 #2888 #2958).

Thanks @AJV20! 🚢

franksong2702 pushed a commit to franksong2702/hermes-webui-fork that referenced this pull request May 27, 2026
# Conflicts:
#	CHANGELOG.md
#	tests/test_webui_prefill_context.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants