[codex] Fix workspace-prefixed user turn leakage - #2145
3 commits merged into
Conversation
|
@swftwolfzyq These have been sitting as draft for 4 days. If you'd like maintainer review, please:
Otherwise we'll close in a few days to keep the queue clean — you can reopen anytime. Both PRs touch interesting surfaces (#2145 the workspace-prefix display leakage, #2146 the release-tag-based update banner), so they're worth a look once you've signaled they're ready. |
|
Ready for review now. I’ve resolved the latest merge conflicts and consider the diff to be in its final shape for maintainer review. |
|
@nesquena Ready signal from @swftwolfzyq received — moving out of CI is green on all three Python versions (3.11/3.12/3.13), |
nesquena
left a comment
There was a problem hiding this comment.
Review — end-to-end ✅ (clean APPROVE, no fix pushed)
What this ships
External-contributor PR (codex/workspace-prefix-display-fix branch — @swftwolfzyq, OpenAI Codex bot). Two narrow fixes in api/streaming.py:
- Workspace-prefix leakage — Stop the internal
[Workspace::v1: /path]metadata tag from leaking into the visible user transcript when a provider failure/retry path echoes an optimistic draft followed by the workspace-prefixed prompt. - Silent-failure detection — Only consider messages after the current user turn when deciding whether the agent produced an assistant reply, so prior assistant messages from earlier turns can't mask a current-turn silent failure.
Surfaces touched: api/streaming.py (+72/-11), tests/test_issue1217_transcript_compaction.py (+91 — 3 new tests).
Traced against upstream hermes-agent
Pulled a fresh tarball. No cross-tool surface — both fixes live entirely in webui-internal message-merge / failure-detection helpers. Agent receives the workspace-prefixed message as before; only the WebUI's display-merge and silent-failure detection are touched. No config.yaml, no session schema, no AIAgent param change.
End-to-end trace
New _looks_like_current_user_turn(msg, msg_text) at api/streaming.py:996-1014:
needle = " ".join(str(msg_text or '').split())
if not needle: return False
text = _message_text(msg.get('content', ''))
candidates = [_strip_workspace_prefix(text, include_legacy=True)] # leading-strip
for pattern in (_WORKSPACE_PREFIX_ANY_RE, _LEGACY_WORKSPACE_PREFIX_ANY_RE):
for match in pattern.finditer(text):
candidates.append(text[match.end():]) # post-prefix tail
return any(" ".join(str(c or '').split()) == needle for c in candidates)- Whitespace-normalized exact-match comparison against the submitted prompt.
- Considers both the leading-prefix-stripped variant (existing path) AND the text after any embedded workspace prefix (new path).
- Returns False for non-user roles, empty prompts, or any text whose post-prefix tail doesn't exactly match.
New regex _WORKSPACE_PREFIX_ANY_RE and _LEGACY_WORKSPACE_PREFIX_ANY_RE at lines 977-978 — same shape as the existing leading-only regex but without the ^ anchor. ReDoS-safe: alternation \\.|[^\]\\] is deterministic (escaped char OR a non-]-non-\ char; no overlap).
**Updated _find_current_user_turn at api/streaming.py:2138 — adds the new check BEFORE the existing strip-then-compare path:
if _looks_like_current_user_turn(msg, msg_text):
return idx
# ... existing _strip_workspace_prefix + exact-match logic ...Shapes the leading-strip caught still hit (since candidates includes the leading-stripped variant first). Shapes that previously fell through to fallback now correctly identify the current user turn.
Updated _merge_display_messages_after_agent_result at lines 2283-2348 — adds _looks_like_current_user_turn checks in three places:
- "current_user_already_checkpointed" guard (whether the previous merged tail already represents the current turn)
- "skip current turn already added" dedupe (when the same identity OR leaked-prefix variant arrives a second time)
- Display-content rewrite — when the message IS the current turn, copy and overwrite
content = msg_textso the visible transcript shows the clean prompt regardless of what the provider echoed.
New _assistant_reply_added_after_current_turn(result_messages, previous_context, msg_text) at lines 2361-2375:
if _messages_have_prefix(result_messages, previous_context):
candidates = result_messages[len(previous_context):] # append-only normal case
else:
current_user_idx = _find_current_user_turn(result_messages, msg_text)
candidates = result_messages[current_user_idx + 1:] if current_user_idx is not None else result_messages
return any(
m.get('role') == 'assistant' and not m.get('_error') and str(m.get('content') or '').strip()
for m in candidates
)Two cases:
- Append-only (typical): candidates = the delta after
previous_context. - Compacted/replayed: locate the current user turn, look at what comes after.
Returns True only if at least one non-error, non-empty assistant message exists in the post-current-turn slice.
Updated call site at lines 4001-4009:
_all_result_messages = result.get('messages') or []
_prev_len = len(_previous_context_messages)
_assistant_added = _assistant_reply_added_after_current_turn(
_all_result_messages,
_previous_context_messages,
msg_text,
)Replaces the old _has_new_assistant_reply(_all_result_messages, _prev_len). _has_new_assistant_reply is still in the module (line 159) and still called from the self-heal path at line 4095 — kept as-is for the heal flow which has a different shape.
Other audit — things that are correct already
Security
- Pure Python text + regex. No SQL, shell, path, XSS surface. The regex
(?:\\.|[^\]\\])+is deterministic (escaped-char OR non-]-non-\char with no overlap); no ReDoS catastrophic backtracking. msg_textand message content come from the WebUI's own session storage; admin-controlled. No untrusted input flows here.
Backward compat
- The new
_looks_like_current_user_turnis OR'd with the existing_message_identity == current_user_keycheck at every merge call site. Messages without leak still match via identity. No regression. _has_new_assistant_replykept intact for the heal path; its tests attests/test_silent_failure_detection.pystill cover its behavior. Existing call sites in tests still work.
False-positive guard
- The whitespace-normalized exact-match requirement means a message matches only when the post-prefix tail EQUALS the prompt verbatim. Cannot accidentally match a different message that happens to contain the workspace prefix.
Display rewrite at line 2348
copy.deepcopy(msg)+display_msg['content'] = msg_text— ensures the visible transcript shows the clean prompt. Original message dict not mutated; only the merged copy. Idempotent.
Test coverage (3 new tests):
test_embedded_workspace_prefixed_current_user_delta_is_deduped— leaked-prefix message in candidates, previous_display already contains the cleaned current turn → merge result equals previous_display (no duplicate added, no leaked tag visible).test_embedded_workspace_prefixed_current_user_delta_displays_clean_prompt— leaked-prefix message + new assistant reply → merge shows clean prompt + assistant, noWorkspace::v1in any output.test_assistant_added_detection_ignores_prior_history— pre-context has an assistant; current-turn delta has only user; helper correctly reportsFalse(silent failure), butTruewhen assistant is appended.
Behavioural harness
12-case Python harness exercising both new helpers:
=== _looks_like_current_user_turn ===
1 clean (no prefix) → True ✓
2 leading workspace prefix → True ✓
3 embedded prefix (bug shape) → True ✓
4 legacy workspace prefix → True ✓
5 different post-prefix content → False ✓ (false-positive guard)
6 assistant role → False ✓ (role guard)
7 empty prompt → False ✓
8 whitespace normalization → True ✓
=== _assistant_reply_added_after_current_turn ===
9 normal flow (assistant added) → True ✓
10 silent failure (no assistant) → False ✓
11 empty-content assistant → False ✓
12 _error-flagged assistant → False ✓
All 12 cases pass. The new helpers correctly identify the bug shape, reject false-positive shapes, and handle all the failure-mode permutations.
Edge-case trace
| Scenario | Expected | Actual |
|---|---|---|
| Clean current-turn user message | identified | ✅ harness 1 |
Leading [Workspace::v1:...] prefix |
identified, displayed clean | ✅ harness 2 |
| Embedded (mid-text) workspace prefix + clean tail | identified, displayed clean | ✅ harness 3 + test 1 |
Legacy [Workspace:...] prefix |
identified | ✅ harness 4 |
| Message with prefix but different content | NOT identified (no false positive) | ✅ harness 5 |
| Assistant role with prompt content | NOT identified | ✅ harness 6 |
| Empty / whitespace prompt | not identified | ✅ harness 7 |
| Whitespace-normalized post-prefix tail | identified | ✅ harness 8 |
| Append-only flow with assistant | reply detected | ✅ harness 9 |
| Append-only flow, NO assistant | silent failure detected | ✅ harness 10 + test 3 |
| Empty-content assistant | not counted as reply | ✅ harness 11 |
_error=True assistant |
not counted | ✅ harness 12 |
| Compacted result (not append-only) | locates current turn, scans post-tail | ✅ _messages_have_prefix fallback |
| Cross-tool: agent sees same workspace-prefixed prompt | unaffected | ✅ webui-only display logic |
Tests
- PR-targeted: 64/64 pass (
test_issue1217_transcript_compaction.py,test_workspace_display_prefix.py,test_issues_373_374_375.py,test_session_save_mode.py). test_silent_failure_detection.py(existing tests for_has_new_assistant_reply): 7/7 pass — the kept-as-is helper still works for the heal path.- Full local suite (Python 3.14): 5713 passed, 63 skipped, 3 xpassed, 0 failed.
- CI: not run (
gh pr checks 2145returned "no checks reported on the 'codex/workspace-prefix-display-fix' branch"). Same situation as PR #2146 — Codex bot branch, GitHub Actions didn't trigger.
Minor observations (non-blocking)
- Heal-path detection NOT updated. The self-heal
_heal_okcheck at line 4095 still uses the older_has_new_assistant_reply(_heal_all_msgs, _prev_len) or _token_sent. If the same workspace-leak shape could appear in heal results, the heal path would mis-detect. PR scopes the fix narrowly to the reported bug path; updating heal could be a follow-up but isn't blocking. _has_new_assistant_replyretained for heal path only — single remaining production caller, plus its own test file. Acceptable; the function isn't dead code._looks_like_current_user_turnis O(n*m) in worst case (finditerover text + comparison per match). In practice n and m are small (single-message text, few prefix matches). Not a hot path.- No CI runs. Same as PR #2146 — Codex-bot branch is older than the workflow-file change window. Maintainer should re-trigger or rebase before merge.
- No CHANGELOG entry. Release agent stamps at merge time per project convention.
- External Codex-bot PR. Code quality is high: clean separation of concerns, narrow scope, good test naming, double-quote/Unicode-aware test strings for the Chinese-prompt bug repro. Reasonable contribution; encourage.
Recommendation
Approved. Both fixes are correct, surgical, and well-tested. The workspace-leak fix has a precise regex shape that's ReDoS-safe and a defensive whitespace-normalized exact-match comparison. The silent-failure detection correctly distinguishes append-only vs compacted result shapes. Behavioural harness confirms all 12 input permutations.
✅ Parked at approval — ready for the release agent's merge/tag pipeline.
f1d399b
… 0.51.85) (#540) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/nesquena/hermes-webui](https://github.com/nesquena/hermes-webui) | patch | `0.51.84` → `0.51.85` | --- ### Release Notes <details> <summary>nesquena/hermes-webui (ghcr.io/nesquena/hermes-webui)</summary> ### [`v0.51.85`](https://github.com/nesquena/hermes-webui/blob/HEAD/CHANGELOG.md#v05185--2026-05-17--Release-BI-stage-378--3-PR-batch--workspace-prefix-display-leakage-fix--release-tag-update-banner--Slice-3a-cancel-control-gate-RFC) [Compare Source](nesquena/hermes-webui@v0.51.84...v0.51.85) ##### Fixed - **PR [#​2145](nesquena/hermes-webui#2145 by [@​swftwolfzyq](https://github.com/swftwolfzyq) — Prevent internal `[Workspace::v1: …]` metadata from leaking into the visible user transcript when a failed provider/retry path echoes an optimistic draft followed by the workspace prefix and the real prompt. Adds `_looks_like_current_user_turn(msg, msg_text)` to match the current human turn even when the internal tag appears mid-text — only when the text after the sentinel exactly matches the submitted prompt — and routes the merge/dedupe/display-normalization paths in `_merge_display_messages_after_agent_result` and `_find_current_user_turn` through it. Replaces `_has_new_assistant_reply` length-delta gating in `_periodic_checkpoint` with the new `_assistant_reply_added_after_current_turn` helper, which slices result messages from the current-turn position before counting assistant deltas — silent/no-response failures are now detected from the current turn alone instead of being masked by prior assistant content. - **PR [#​2146](nesquena/hermes-webui#2146 by [@​swftwolfzyq](https://github.com/swftwolfzyq) — Track WebUI update checks against the latest published release tag instead of every commit on the upstream branch, so operators who only want released versions stop seeing noisy update banners for post-release development commits. Falls back to branch-based detection when no release tags are available. Splits `git describe --dirty` into a fast base describe plus a bounded dirty probe so WSL-mounted workspaces never block version detection on a slow `--dirty` walk, keeping the base version visible even if the dirty probe times out. ##### Documentation - **PR [#​2469](nesquena/hermes-webui#2469 by [@​Michaelyklam](https://github.com/Michaelyklam) (refs [#​1925](nesquena/hermes-webui#1925)) — Advance the runtime-adapter RFC after the Slice 2 seam shipped by marking Slice 2 complete and defining the first Slice 3a cancel-control gate. The new gate scopes Stop Generation through `RuntimeAdapter.cancel_run(...)` only, pins behavior-preserving cancellation, journal/status coherence, idempotent duplicate cancel, and explicit non-goals for approval/clarify, queue/goal, runner/sidecar, and public chat-start response changes. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/homelab/pulls/540
Summary
[Workspace::v1: ...]metadata from leaking into the visible user transcript when a provider failure/retry path echoes draft text before the workspace-prefixed promptRoot cause
WebUI sends Hermes a workspace-prefixed user message for model context while the UI transcript should keep only the human text. In failed provider paths, returned messages can contain a partial optimistic draft followed by the internal workspace prefix and the full prompt. The existing dedupe logic only stripped prefixes at the beginning of a user message, so the internal tag could be persisted visibly. The no-response guard also checked all result messages, allowing prior assistant replies to mask a failed current turn.
Tests
~/.local/bin/uv run --with pytest pytest tests/test_issue1217_transcript_compaction.py tests/test_workspace_display_prefix.py tests/test_issues_373_374_375.py tests/test_session_save_mode.py tests/test_issue1361_cancel_data_loss.py -q(71 passed)~/.local/bin/uv run --with pytest pytest tests/test_issue1217_transcript_compaction.py tests/test_workspace_display_prefix.py -q(13 passed)python -m py_compile api/streaming.py