David/HERMES-GIELIE
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix CLI session patch diff rendering

Browse source
This commit is contained in:
Frank Song 2026-05-10 20:44:34 +08:00
parent e4a9c5b7f5
commit e64e02479f
3 changed files with 383 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -49,6 +49,8 @@ Three nice-to-have polish items called out by Opus that don't block this release
### Fixed
- **bug(cli-import): patch diffs invisible in imported CLI sessions** ([#1824](https://github.com/nesquena/hermes-webui/issues/1824)). Historical CLI sessions that predate session-level `tool_calls` fall back to reconstructing tool cards from per-message metadata in `static/ui.js`. That fallback truncated tool results to 200 chars and only showed the first 120 chars of tool arguments, so `apply_patch`/edit diffs recorded with `verbosity=all` could disappear behind a generic `Success` result. The renderer now preserves diff-like tool outputs, promotes `apply_patch`/edit payloads into the tool-card snippet when the result is non-diff, and labels long diff expanders as `Show diff`. Adds regression coverage for both the API payload preservation and the renderer fallback.
- **bug(profile/mcp): non-default profile MCP servers never load** ([#1968](https://github.com/nesquena/hermes-webui/issues/1968)). `_run_agent_streaming` called `discover_mcp_tools()` ~100 lines BEFORE the per-session `os.environ['HERMES_HOME'] = _profile_home` mutation, so MCP discovery always read the default profile's `~/.hermes/config.yaml` regardless of which profile the session was stamped with. Result: switching profiles in the WebUI dropdown was effectively cosmetic for MCP — non-default profiles never registered their stdio (npx/node) MCP servers. Fix relocates the `discover_mcp_tools()` call past the `_ENV_LOCK` env-mutation block so `get_hermes_home()` resolves to the session's actual profile home. Adds 4 static regression tests (`tests/test_issue1968_mcp_profile_discovery.py`) pinning the call ordering, lock-release placement, single call site, and try/except wrapping. **Caveat (out of scope, agent-side):** `_servers` in `tools/mcp_tool.py` is a process-global dict keyed only by server name, so concurrent use of multiple non-default profiles in the same WebUI process still has a "first profile wins per name" issue. Fully fixing that requires keying `_servers` by `(profile_home, name)` upstream in hermes-agent. This PR ships layer 1 only.
## [v0.51.31] — 2026-05-09 — Release H (12-PR contributor batch: image-mode + race fixes + composer drafts + locale parity)

151
static/ui.js
View file

@ -4681,6 +4681,113 @@ function clearMessageRenderCache(){
_sessionHtmlCacheSid=null;
}
function _clipCliToolSnippet(text, maxLen=20000){
const s=String(text||'');
if(s.length<=maxLen) return s;
return `${s.slice(0,maxLen)}\n\n... truncated ${s.length-maxLen} chars ...`;
}
function _cliToolResultText(raw){
const s=String(raw||'');
try{
const rd=JSON.parse(s);
if(rd && typeof rd==='object'){
for(const key of ['output','result','error','content','diff','patch']){
if(Object.prototype.hasOwnProperty.call(rd,key)){
const v=rd[key];
if(v==null) return '';
return typeof v==='string' ? v : JSON.stringify(v,null,2);
}
}
}
}catch(e){}
return s;
}
function _cliLooksLikePatchDiff(text){
const s=String(text||'');
if(!s) return false;
if(/\*\*\* Begin Patch/.test(s)) return true;
if(/^diff --git /m.test(s)) return true;
if(/^@@\s/m.test(s)) return true;
if(/(^|\n)---\s+/.test(s) && /(^|\n)\+\+\+\s+/.test(s)) return true;
return false;
}
function _cliToolResultSnippet(raw){
const fullText=_cliToolResultText(raw);
if(_cliLooksLikePatchDiff(fullText)) return _clipCliToolSnippet(fullText);
return String(fullText||'').slice(0,200);
}
function _prefixedCliDiffLines(prefix, value){
return String(value||'').split('\n').map(line=>`${prefix}${line}`).join('\n');
}
function _firstOwnedValue(obj, keys){
for(const key of keys){
if(obj && Object.prototype.hasOwnProperty.call(obj,key)) return obj[key];
}
return undefined;
}
function _cliPatchSnippetFromArgs(name, args){
if(!args || typeof args!=='object') return '';
const toolName=String(name||'').toLowerCase();
for(const key of ['patch','diff']){
const v=args[key];
if(typeof v==='string' && v.trim()) return _clipCliToolSnippet(v);
}
for(const key of ['input','content']){
const v=args[key];
if(typeof v==='string' && _cliLooksLikePatchDiff(v)) return _clipCliToolSnippet(v);
}
const isEditLike=toolName==='apply_patch'
|| toolName==='patch'
|| toolName.includes('edit')
|| toolName==='replace'
|| toolName==='str_replace';
if(!isEditLike) return '';
const oldValue=_firstOwnedValue(args,['old_string','old_str','old','before']);
const newValue=_firstOwnedValue(args,['new_string','new_str','new','after']);
if(oldValue!==undefined || newValue!==undefined){
const path=String(_firstOwnedValue(args,['file_path','path','filename'])||'');
const lines=[];
if(path) lines.push(path);
if(oldValue!==undefined) lines.push(_prefixedCliDiffLines('-', oldValue));
if(newValue!==undefined) lines.push(_prefixedCliDiffLines('+', newValue));
return _clipCliToolSnippet(lines.join('\n'));
}
if(Array.isArray(args.edits)){
const path=String(_firstOwnedValue(args,['file_path','path','filename'])||'');
const chunks=[];
if(path) chunks.push(path);
args.edits.slice(0,5).forEach(edit=>{
if(!edit || typeof edit!=='object') return;
const before=_firstOwnedValue(edit,['old_string','old_str','old','before']);
const after=_firstOwnedValue(edit,['new_string','new_str','new','after']);
if(before!==undefined) chunks.push(_prefixedCliDiffLines('-', before));
if(after!==undefined) chunks.push(_prefixedCliDiffLines('+', after));
});
if(chunks.length) return _clipCliToolSnippet(chunks.join('\n'));
}
return '';
}
function _cliToolCardSnippet(resultSnippet, patchSnippet){
if(_cliLooksLikePatchDiff(resultSnippet)) return resultSnippet;
if(!patchSnippet) return resultSnippet || '';
const result=String(resultSnippet||'').trim();
if(!result) return patchSnippet;
const generic=/^(success|ok|done|done\.|exit code: 0)$/i.test(result);
if(generic) return patchSnippet;
return `${resultSnippet}\n\n${patchSnippet}`;
}
function _cliToolCardHasDiffSnippet(resultSnippet, patchSnippet){
return !!patchSnippet || _cliLooksLikePatchDiff(resultSnippet);
}
function _captureMessageScrollSnapshot(){
const el=$('messages');
if(!el) return null;
@ -5070,20 +5177,12 @@ function renderMessages(options){
// fallback-built cards carry their result snippet (not just the command).
// Without this step CLI-origin sessions reload with empty tool cards.
const resultsByTid={};
const _snipFromRaw=(raw)=>{
const s=String(raw||'');
try{
const rd=JSON.parse(s);
if(rd && typeof rd==='object') return String(rd.output||rd.result||rd.error||s).slice(0,200);
}catch(e){}
return s.slice(0,200);
};
S.messages.forEach(m=>{
if(!m) return;
// OpenAI / Hermes CLI format: role=tool with tool_call_id
if(m.role==='tool'){
const tid=m.tool_call_id||m.tool_use_id||'';
if(tid) resultsByTid[tid]=_snipFromRaw(m.content);
if(tid) resultsByTid[tid]=_cliToolResultSnippet(m.content);
return;
}
// Anthropic format: tool_result blocks inside a user message content array
@ -5095,7 +5194,7 @@ function renderMessages(options){
const raw=typeof p.content==='string'?p.content
:Array.isArray(p.content)?p.content.map(c=>c&&c.text?c.text:'').join('')
:'';
resultsByTid[tid]=_snipFromRaw(raw);
resultsByTid[tid]=_cliToolResultSnippet(raw);
});
}
});
@ -5109,10 +5208,20 @@ function renderMessages(options){
const name=fn.name||tc.name||'tool';
let args={};
try{ args=JSON.parse(fn.arguments||'{}'); }catch(e){}
const tid=tc.id||tc.call_id||'';
const patchSnippet=_cliPatchSnippetFromArgs(name,args);
const resultSnippet=resultsByTid[tid]||'';
let argsSnap={};
Object.keys(args).slice(0,4).forEach(k=>{ const v=String(args[k]); argsSnap[k]=v.slice(0,120)+(v.length>120?'...':''); });
const tid=tc.id||tc.call_id||'';
derived.push({name,snippet:resultsByTid[tid]||'',tid,assistant_msg_idx:rawIdx,args:argsSnap,done:true});
derived.push({
name,
snippet:_cliToolCardSnippet(resultSnippet,patchSnippet),
is_diff:_cliToolCardHasDiffSnippet(resultSnippet,patchSnippet),
tid,
assistant_msg_idx:rawIdx,
args:argsSnap,
done:true,
});
});
// Anthropic format: tool_use blocks inside assistant content array
if(Array.isArray(m.content)){
@ -5120,12 +5229,22 @@ function renderMessages(options){
if(!p||typeof p!=='object'||p.type!=='tool_use') return;
const name=p.name||'tool';
const args=p.input||{};
const tid=p.id||'';
const patchSnippet=_cliPatchSnippetFromArgs(name,args);
const resultSnippet=resultsByTid[tid]||'';
const argsSnap={};
if(args && typeof args==='object'){
Object.keys(args).slice(0,4).forEach(k=>{ const v=String(args[k]); argsSnap[k]=v.slice(0,120)+(v.length>120?'...':''); });
}
const tid=p.id||'';
derived.push({name,snippet:resultsByTid[tid]||'',tid,assistant_msg_idx:rawIdx,args:argsSnap,done:true});
derived.push({
name,
snippet:_cliToolCardSnippet(resultSnippet,patchSnippet),
is_diff:_cliToolCardHasDiffSnippet(resultSnippet,patchSnippet),
tid,
assistant_msg_idx:rawIdx,
args:argsSnap,
done:true,
});
});
}
});
@ -5347,6 +5466,8 @@ function buildToolCard(tc){
}
}
const hasMore=tc.snippet&&tc.snippet.length>displaySnippet.length;
const moreLabel=tc.is_diff?'Show diff':'Show more';
const lessLabel=tc.is_diff?'Hide diff':'Show less';
const runIndicator=tc.done===false?'<span class="tool-card-running-dot"></span>':'';
const isSubagent=tc.name==='subagent_progress';
const isDelegation=tc.name==='delegate_task';
@ -5370,7 +5491,7 @@ function buildToolCard(tc){
}</div>`:''}
${displaySnippet?`<div class="tool-card-result">
<pre>${esc(displaySnippet)}</pre>
${hasMore?`<button class="tool-card-more" data-full="${esc(tc.snippet||'').replace(/"/g,'&quot;')}" data-short="${esc(displaySnippet||'').replace(/"/g,'&quot;')}" onclick="event.stopPropagation();const p=this.previousElementSibling;const full=this.dataset.full;const short=this.dataset.short;p.textContent=p.textContent===short?full:short;this.textContent=p.textContent===short?'Show more':'Show less'">Show more</button>`:''}
${hasMore?`<button class="tool-card-more" data-full="${esc(tc.snippet||'').replace(/"/g,'&quot;')}" data-short="${esc(displaySnippet||'').replace(/"/g,'&quot;')}" data-more-label="${esc(moreLabel)}" data-less-label="${esc(lessLabel)}" onclick="event.stopPropagation();const p=this.previousElementSibling;const full=this.dataset.full;const short=this.dataset.short;p.textContent=p.textContent===short?full:short;this.textContent=p.textContent===short?this.dataset.moreLabel:this.dataset.lessLabel">${esc(moreLabel)}</button>`:''}
</div>`:''}
</div>`:''}
</div>`;

245
tests/test_issue1824_cli_patch_diff_rendering.py Normal file
View file

@ -0,0 +1,245 @@
import json
import re
import sqlite3
import subprocess
import textwrap
from pathlib import Path
ROOT = Path(__file__).resolve().parents[1]
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
COMPACT_UI = re.sub(r"\s+", "", UI_JS)
def test_cli_tool_result_diff_snippet_is_not_cut_to_200_chars():
"""Diff-like CLI tool results should reach the existing tool-card expander."""
assert "function _cliToolResultSnippet" in UI_JS
assert "function _cliLooksLikePatchDiff" in UI_JS
assert r"\*\*\* Begin Patch" in UI_JS
assert "diff --git" in UI_JS
assert (
"if(_cliLooksLikePatchDiff(fullText))return_clipCliToolSnippet(fullText);"
in COMPACT_UI
)
assert "returnString(fullText||'').slice(0,200);" in COMPACT_UI
def test_cli_tool_fallback_promotes_apply_patch_args_to_tool_card_snippet():
"""A successful apply_patch result may only say 'Success'; keep the patch visible."""
assert "function _cliPatchSnippetFromArgs" in UI_JS
assert "toolName==='apply_patch'" in COMPACT_UI
assert "'old_string'" in UI_JS
assert "'new_string'" in UI_JS
assert "constpatchSnippet=_cliPatchSnippetFromArgs(name,args);" in COMPACT_UI
assert "snippet:_cliToolCardSnippet(resultSnippet,patchSnippet)" in COMPACT_UI
assert "is_diff:_cliToolCardHasDiffSnippet(resultSnippet,patchSnippet)" in COMPACT_UI
def test_diff_tool_cards_use_show_diff_expander_label():
assert "const moreLabel=tc.is_diff?'Show diff':'Show more';" in UI_JS
assert "const lessLabel=tc.is_diff?'Hide diff':'Show less';" in UI_JS
assert 'data-more-label="${esc(moreLabel)}"' in UI_JS
def _function_source(src: str, name: str) -> str:
match = re.search(rf"function\s+{re.escape(name)}\s*\(", src)
assert match, f"{name}() not found"
brace = src.find("{", match.end())
assert brace != -1, f"{name}() has no body"
depth = 1
i = brace + 1
in_string = None
escaped = False
in_line_comment = False
in_block_comment = False
while i < len(src) and depth:
ch = src[i]
nxt = src[i + 1] if i + 1 < len(src) else ""
if in_line_comment:
if ch == "\n":
in_line_comment = False
i += 1
continue
if in_block_comment:
if ch == "*" and nxt == "/":
in_block_comment = False
i += 2
continue
i += 1
continue
if in_string:
if escaped:
escaped = False
elif ch == "\\":
escaped = True
elif ch == in_string:
in_string = None
i += 1
continue
if ch == "/" and nxt == "/":
in_line_comment = True
i += 2
continue
if ch == "/" and nxt == "*":
in_block_comment = True
i += 2
continue
if ch in "'\"`":
in_string = ch
i += 1
continue
if ch == "{":
depth += 1
elif ch == "}":
depth -= 1
i += 1
assert depth == 0, f"{name}() body did not close"
return src[match.start() : i]
def test_rendered_apply_patch_tool_card_html_contains_diff_lines():
"""Drive the actual snippet helpers and buildToolCard() through Node."""
function_names = [
"_clipCliToolSnippet",
"_cliToolResultText",
"_cliLooksLikePatchDiff",
"_cliToolResultSnippet",
"_prefixedCliDiffLines",
"_firstOwnedValue",
"_cliPatchSnippetFromArgs",
"_cliToolCardSnippet",
"_cliToolCardHasDiffSnippet",
"buildToolCard",
]
functions = "\n".join(_function_source(UI_JS, name) for name in function_names)
script = textwrap.dedent(
f"""
function esc(s){{return String(s||'').replace(/[&<>]/g,c=>({{'&':'&amp;','<':'&lt;','>':'&gt;'}}[c]));}}
function li(){{return '';}}
function toolIcon(){{return '';}}
function _toolDisplayName(tc){{return tc.name||'tool';}}
const document={{
createElement(){{return {{className:'', innerHTML:''}};}}
}};
{functions}
const longPatch = [
'*** Begin Patch',
'*** Update File: app.py',
'@@',
'-old',
'+new',
...Array.from({{length: 150}}, (_, i) => '+line ' + i),
'*** End Patch'
].join('\\n');
const resultSnippet = _cliToolResultSnippet(JSON.stringify({{output:'Success'}}));
const patchSnippet = _cliPatchSnippetFromArgs('apply_patch', {{patch: longPatch}});
const row = buildToolCard({{
name: 'apply_patch',
snippet: _cliToolCardSnippet(resultSnippet, patchSnippet),
is_diff: _cliToolCardHasDiffSnippet(resultSnippet, patchSnippet),
args: {{patch: '(shown in diff)'}},
done: true
}});
const errorSnippet = _cliToolCardSnippet('Patch failed: context not found', patchSnippet);
process.stdout.write(JSON.stringify({{html: row.innerHTML, errorSnippet}}));
"""
)
proc = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
payload = json.loads(proc.stdout)
html = payload["html"]
assert "-old" in html
assert "+new" in html
assert "Show diff" in html
assert "Patch failed: context not found" in payload["errorSnippet"]
assert "-old" in payload["errorSnippet"]
def _make_state_db(path: Path) -> None:
patch = "\n".join(
[
"*** Begin Patch",
"*** Update File: app.py",
"@@",
"-old",
"+new",
"*** End Patch",
]
)
tool_calls = [
{
"id": "call_patch",
"type": "function",
"function": {
"name": "apply_patch",
"arguments": json.dumps({"patch": patch}),
},
}
]
conn = sqlite3.Connection(str(path))
try:
conn.executescript(
"""
CREATE TABLE messages (
id INTEGER PRIMARY KEY AUTOINCREMENT,
session_id TEXT,
role TEXT,
content TEXT,
timestamp TEXT,
tool_call_id TEXT,
tool_calls TEXT,
tool_name TEXT
);
"""
)
conn.execute(
"""
INSERT INTO messages (session_id, role, content, timestamp, tool_calls)
VALUES (?, ?, ?, ?, ?)
""",
("issue1824", "assistant", "", "2026-01-01T00:00:01Z", json.dumps(tool_calls)),
)
conn.execute(
"""
INSERT INTO messages (session_id, role, content, timestamp, tool_call_id, tool_name)
VALUES (?, ?, ?, ?, ?, ?)
""",
(
"issue1824",
"tool",
json.dumps({"output": "Success"}),
"2026-01-01T00:00:02Z",
"call_patch",
"apply_patch",
),
)
conn.commit()
finally:
conn.close()
def test_cli_session_reader_preserves_apply_patch_metadata(tmp_path, monkeypatch):
"""The API payload should keep tool_calls/tool rows for the UI renderer."""
_make_state_db(tmp_path / "state.db")
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
import api.profiles
from api.models import get_cli_session_messages
monkeypatch.setattr(api.profiles, "get_active_hermes_home", lambda: str(tmp_path))
messages = get_cli_session_messages("issue1824")
assert [m["role"] for m in messages] == ["assistant", "tool"]
assistant = messages[0]
assert assistant["tool_calls"][0]["function"]["name"] == "apply_patch"
args = json.loads(assistant["tool_calls"][0]["function"]["arguments"])
assert "*** Begin Patch" in args["patch"]
assert "-old" in args["patch"]
assert "+new" in args["patch"]
tool = messages[1]
assert tool["tool_call_id"] == "call_patch"
assert tool["tool_name"] == "apply_patch"
assert tool["name"] == "apply_patch"
assert json.loads(tool["content"])["output"] == "Success"