Skip to content

fix: resolve duplicate TERMINAL_CWD in _set_thread_env call - #1271

Closed
wtj-0527 wants to merge 1 commit into
nesquena:masterfrom
wtj-0527:fix/streaming-terminal-cwd-error
Closed

wtj-0527 wants to merge 1 commit into
nesquena:masterfrom
wtj-0527:fix/streaming-terminal-cwd-error

Conversation

@wtj-0527

Copy link
Copy Markdown

Problem

When terminal.cwd is configured in config.yaml, get_profile_runtime_env() includes TERMINAL_CWD in the returned dict. Then streaming.py explicitly passes TERMINAL_CWD=str(s.workspace), causing:

TypeError: api.config._set_thread_env() got multiple values for keyword argument 'TERMINAL_CWD'

Root Cause

_profile_runtime_env dict may contain TERMINAL_CWD from profile config (via _TERMINAL_ENV_MAPPINGS['cwd'] = 'TERMINAL_CWD'), which conflicts with the explicit TERMINAL_CWD parameter passed to _set_thread_env().

Solution

Remove TERMINAL_CWD from _profile_runtime_env before calling _set_thread_env to avoid the duplicate keyword argument error. This ensures the session's workspace path is always used.

Testing

  • Verified no TypeError in WebUI logs after fix
  • Service starts and runs correctly
  • Streaming API works without errors

Problem:
When terminal.cwd is configured in config.yaml, get_profile_runtime_env()
includes TERMINAL_CWD in the returned dict. Then streaming.py explicitly
passes TERMINAL_CWD=str(s.workspace), causing TypeError:
'set_thread_env() got multiple values for keyword argument TERMINAL_CWD'

Root Cause:
_profile_runtime_env dict may contain TERMINAL_CWD from profile config
(via terminal.cwd mapping), which conflicts with the explicit TERMINAL_CWD
parameter passed to _set_thread_env().

Solution:
Remove TERMINAL_CWD from _profile_runtime_env before calling _set_thread_env
to avoid the duplicate keyword argument error. This ensures the session's
workspace path is always used.

Testing:
- Verified no TypeError in WebUI logs after fix
- Service starts and runs correctly
- Streaming API works without errors
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks for the targeted fix, @KingBoyAndGirl!

What this does: Removes TERMINAL_CWD from _profile_runtime_env (via pop()) before calling _set_thread_env(), preventing the TypeError that fires when terminal.cwd is configured in a profile's config.yaml.

Note on approach vs. sibling PRs: There are now three PRs addressing the same root cause in issue #1270:

The pop() approach here is the most minimal and lowest-risk fix. The downside noted in the PR body is that the same fix would need to be repeated for each conflicting key (HERMES_EXEC_ASK, HERMES_SESSION_KEY, HERMES_HOME as called out in issue #1270). PR #1266's merge approach handles that more generically.

Recommendation for maintainer: Pick one of the three PRs to merge and close the other two to avoid a double-apply. All three are correct fixes; the choice is a style/scope preference.

Leaving for maintainer review and merge decision.

@wtj-0527

Copy link
Copy Markdown
Author

Thanks for the summary, @nesquena-hermes! 🙏

I've noted that there are three PRs addressing the same issue (#1270):

My stance: I'm happy with whatever the maintainer decides! Each approach is correct; it's a style/scope preference.

If #1266 or #1265 is chosen, just let me know and I'll close this PR. The goal is to get the fix merged, not to push a specific implementation. 😊

Maintainers: please pick whichever approach you prefer!

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks for flagging the three-PR situation, @KingBoyAndGirl — that context is helpful for the maintainer.

Quick summary for whoever merges:

The merge approach (#1266) is the most defensive against future duplicate-key bugs. The pop() approach here is safe and correct for the current case. The helper approach is the most structural refactor.

Once one of these is merged, the other two should be closed. The maintainer's call on which shape they prefer.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Hold — superseded by #1266

Thanks for the fix, @KingBoyAndGirl! The .pop('TERMINAL_CWD', None) approach is correct for unblocking the immediate TypeError. However, PR #1266 (from @hi-friday) solves the same problem at a higher level: it introduces _build_agent_thread_env() which merges all env keys into a single dict before _set_thread_env, so duplicates are impossible by construction — not just pruned case-by-case.

Since #1266 fully subsumes the fix in this PR and includes tests, we'll route through #1266 instead. Closing this one to keep the diff set clean.

We'll close this PR when #1266 lands. Thanks again for the sharp diagnosis!

nesquena-hermes added a commit that referenced this pull request Apr 30, 2026
## Release v0.50.240

Batch release of 13 PRs that passed full triage + code review + test suite (3199 tests, 0 failures).

---

### Added

- **Compact tool activity mode** (`simplified_tool_calling`, default on) — groups tool calls and thinking traces into a single collapsed "Activity" disclosure card per assistant turn. Also adds a new **Calm Console** theme with earth/slate palette and serif prose. @Michaelyklam#1282
- **PDF first-page preview** — `MEDIA:` `.pdf` files render a canvas thumbnail via PDF.js CDN (4 MB cap). **HTML sandbox iframe** — `.html`/`.htm` files render inline in a sandboxed `<iframe srcdoc>` (256 KB cap). 10 i18n keys × 7 locales. @bergeouss#1280, closes #480 #482
- **Inline Excalidraw diagram preview** — `.excalidraw` files render as pure SVG (no external deps; rectangles, ellipses, diamonds, text, lines, arrows, freehand; 512 KB cap). @bergeouss#1279, closes #479
- **Inline CSV table rendering** — fenced `csv` blocks and `MEDIA:` CSV files render as scrollable HTML tables with auto-separator detection. @bergeouss#1277, closes #485
- **Inline SVG, audio, and video rendering** — SVG as `<img>`, audio as `<audio controls>`, video as `<video controls>`. @bergeouss#1276, closes #481
- **Batch session select mode** — multi-select sessions for bulk Archive/Delete/Move. 11 i18n keys × 7 locales. @bergeouss#1275, closes #568
- **Collapsible skill category headers** — click to collapse/expand without re-render; state persists across filter cycles. @bergeouss#1281
- **`providers.only_configured` setting** — opt-in flag to restrict the model picker to explicitly configured providers. @KingBoyAndGirl — #1268
- **OpenCode Go model catalog** — adds Kimi K2.6, DeepSeek V4 Pro/Flash, MiMo V2.5/Pro, Qwen3.6/3.5 Plus. @nesquena-hermes#1284, closes #1269

### Fixed

- **Profile `TERMINAL_CWD` TypeError** — `_build_agent_thread_env()` helper merges env before `_set_thread_env()` call. @hi-friday#1266
- **Service worker subpath cache bypass** — regex now matches `/api/*` under any mount prefix. @Michaelyklam#1278
- **SSE client disconnect leaks** — `TimeoutError`/`OSError` treated as clean disconnects; server backlog 64, threads daemonized; session list renders before saved-session restore. @KayZz69#1267
- **i18n locale corrections** — Korean MCP strings (23), Chinese MCP strings (23), zh-Hant missing keys (41), de missing keys (229). @bergeouss#1274, closes #1273

---

### Test results

```
3199 passed, 2 skipped, 3 xpassed in 72.79s
```

### PRs on hold (not included)

#1265 (draft), #1271 (superseded by #1266), #1272 (skipped XSS tests), #1232 (partial test run), #1222 (review questions open), #1134 (live-server tests), #1132 (superseded by #1134), #1108 (negative UX review), #1084 (empty description)
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

PR #1266 (which supersedes this) shipped in v0.50.240. Closing.

SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
## Release v0.50.240

Batch release of 13 PRs that passed full triage + code review + test suite (3199 tests, 0 failures).

---

### Added

- **Compact tool activity mode** (`simplified_tool_calling`, default on) — groups tool calls and thinking traces into a single collapsed "Activity" disclosure card per assistant turn. Also adds a new **Calm Console** theme with earth/slate palette and serif prose. @Michaelyklamnesquena#1282
- **PDF first-page preview** — `MEDIA:` `.pdf` files render a canvas thumbnail via PDF.js CDN (4 MB cap). **HTML sandbox iframe** — `.html`/`.htm` files render inline in a sandboxed `<iframe srcdoc>` (256 KB cap). 10 i18n keys × 7 locales. @bergeoussnesquena#1280, closes nesquena#480 nesquena#482
- **Inline Excalidraw diagram preview** — `.excalidraw` files render as pure SVG (no external deps; rectangles, ellipses, diamonds, text, lines, arrows, freehand; 512 KB cap). @bergeoussnesquena#1279, closes nesquena#479
- **Inline CSV table rendering** — fenced `csv` blocks and `MEDIA:` CSV files render as scrollable HTML tables with auto-separator detection. @bergeoussnesquena#1277, closes nesquena#485
- **Inline SVG, audio, and video rendering** — SVG as `<img>`, audio as `<audio controls>`, video as `<video controls>`. @bergeoussnesquena#1276, closes nesquena#481
- **Batch session select mode** — multi-select sessions for bulk Archive/Delete/Move. 11 i18n keys × 7 locales. @bergeoussnesquena#1275, closes nesquena#568
- **Collapsible skill category headers** — click to collapse/expand without re-render; state persists across filter cycles. @bergeoussnesquena#1281
- **`providers.only_configured` setting** — opt-in flag to restrict the model picker to explicitly configured providers. @KingBoyAndGirl — nesquena#1268
- **OpenCode Go model catalog** — adds Kimi K2.6, DeepSeek V4 Pro/Flash, MiMo V2.5/Pro, Qwen3.6/3.5 Plus. @nesquena-hermesnesquena#1284, closes nesquena#1269

### Fixed

- **Profile `TERMINAL_CWD` TypeError** — `_build_agent_thread_env()` helper merges env before `_set_thread_env()` call. @hi-fridaynesquena#1266
- **Service worker subpath cache bypass** — regex now matches `/api/*` under any mount prefix. @Michaelyklamnesquena#1278
- **SSE client disconnect leaks** — `TimeoutError`/`OSError` treated as clean disconnects; server backlog 64, threads daemonized; session list renders before saved-session restore. @KayZz69nesquena#1267
- **i18n locale corrections** — Korean MCP strings (23), Chinese MCP strings (23), zh-Hant missing keys (41), de missing keys (229). @bergeoussnesquena#1274, closes nesquena#1273

---

### Test results

```
3199 passed, 2 skipped, 3 xpassed in 72.79s
```

### PRs on hold (not included)

nesquena#1265 (draft), nesquena#1271 (superseded by nesquena#1266), nesquena#1272 (skipped XSS tests), nesquena#1232 (partial test run), nesquena#1222 (review questions open), nesquena#1134 (live-server tests), nesquena#1132 (superseded by nesquena#1134), nesquena#1108 (negative UX review), nesquena#1084 (empty description)
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