Skip to content

fix: support CLI sessions in /api/list file browser - #204

Merged
nesquena-hermes merged 3 commits into
nesquena:masterfrom
iRonin:fix/workspace-list-cli-sessions
Apr 10, 2026
Merged

fix: support CLI sessions in /api/list file browser#204
nesquena-hermes merged 3 commits into
nesquena:masterfrom
iRonin:fix/workspace-list-cli-sessions

Conversation

@iRonin

@iRonin iRonin commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Problem

When a CLI session is selected in the Web UI, the workspace file browser is empty. The /api/list endpoint returns 404 because it only checks WebUI in-memory sessions, not CLI sessions imported from state.db.

Fix

_handle_list_dir() now falls back to get_cli_sessions() to find the workspace path for CLI sessions that aren't loaded in WebUI memory. This matches the existing pattern used in /api/session GET and /api/sessions list endpoints.

Impact

  • File browser now works for CLI sessions
  • No change to WebUI session behavior
  • 17 lines added, 3 removed

Closes the bug where workspace pane showed no files for CLI sessions.

iRonin added 3 commits April 9, 2026 12:05
@iRonin
feat: optional HTTPS/TLS support via cert and key env vars
88d12b5 
Add optional HTTPS support controlled by two env vars:
  HERMES_WEBUI_TLS_CERT=/path/to/cert.pem
  HERMES_WEBUI_TLS_KEY=/path/to/key.pem

- Wraps server socket with ssl.SSLContext (min TLSv1.2)
- Dynamic scheme detection for startup messages (http:// vs https://)
- Graceful fallback to HTTP if cert loading fails — server never crashes
  due to bad TLS config, just prints a warning and continues
- Auth cookie Secure flag already set when HTTPS is detected via getpeercert
- 6 end-to-end tests: config flags, HTTPS handshake, HTTP still works,
  fallback on bad paths

Addresses nesquena#191 (HTTPS support issue).
@iRonin
fix: use current branch upstream for update checks, not repo default …
e157fb5 
…branch

The update checker in api/updates.py always compared HEAD against
origin/master (or origin/main), which produced false 'N updates
available' alerts when the user is on a feature branch and master has
moved forward with unrelated commits.

Now uses git rev-parse --abbrev-ref @{upstream} to get the current
branch's tracking branch for both the behind-count check and the
apply-update pull command. Falls back to the default branch if no
upstream is set (brand-new local branch with no tracking config).

Fixes nesquena#200.
@iRonin
fix: support CLI sessions in /api/list file browser
87c0310 
_handle_list_dir() only checked WebUI in-memory sessions, returning
'Session not found' for CLI sessions imported from the agent's state.db.
Now falls back to get_cli_sessions() to find the workspace path for
CLI sessions that aren't loaded in WebUI memory.

Fixes: workspace pane showing empty for CLI sessions.
@nesquena-hermes

nesquena-hermes commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this fix — the root cause is correct and the approach matches the existing pattern in the codebase.

What's happening

_handle_list_dir() was calling get_session(sid) which only checks the WebUI in-memory SESSIONS dict. CLI sessions imported from state.db are never loaded into that dict, so the lookup always returns None / raises KeyError, which routes to a 404.

Fix assessment

The fallback to get_cli_sessions() is exactly the right fix and matches how /api/session GET and /api/sessions list already handle CLI sessions. The pattern is consistent across the codebase.

A few things that look good:

  • The fallback only runs when get_session(sid) returns None, so WebUI sessions are unaffected
  • get_cli_sessions() is already tested and used elsewhere — no new infrastructure
  • The workspace path is extracted from the CLI session dict using the same key the other endpoints use

One thing to verify: if a CLI session has no workspace key set (e.g., an old session created before workspace tracking was added), the fallback should return a sensible error rather than a Python KeyError. Worth checking that the fallback path handles a missing workspace key gracefully.

Closes #203. Ready for maintainer review.

@nesquena-hermes nesquena-hermes mentioned this pull request Apr 9, 2026
Closed
@nesquena

nesquena commented Apr 9, 2026

Copy link
Copy Markdown
Owner

Full Review: PR #204 — workspace file browser for CLI sessions

Thanks @iRonin! Real bug, correct fix pattern.

Security Audit

Clean. The fallback path uses get_cli_sessions() (which reads from state.db) to find the workspace path, then passes it to list_dir() which already uses safe_resolve_ws() for path traversal protection. No new attack surface.

Code Review

The fix matches the existing fallback pattern used in /api/session GET and /api/sessions list — when a session isn't found in WebUI memory, check CLI sessions from state.db. The workspace variable is extracted before the list_dir() call, keeping the control flow clean.

The for cs in get_cli_sessions() loop is O(n) over all CLI sessions, but this is only hit when the session ID isn't in WebUI memory (rare path), and CLI session lists are typically small. Fine.

Stacked Branch Issue

This PR includes changes from both PR #199 (TLS) and PR #201 (update checker) — it was branched from #201 which was branched from #199. The diff shows 277 additions but only ~17 are the actual CLI sessions fix in api/routes.py.

Files that don't belong to this PR:

Recommended merge order: #199 first, then rebase #201, then rebase #204. Each PR should only contain its own changes.

Test Results

512 passed, 0 failed, 41 skipped. No regressions.

Verdict

The CLI sessions fix itself is approved — correct and safe. But please rebase this onto master after PRs #199 and #201 are merged to isolate the actual 17-line change. As-is, merging this would silently ship TLS support and the update checker fix without proper individual review.

nesquena-hermes pushed a commit that referenced this pull request Apr 9, 2026
@nesquena
fix(cli-sessions): guard empty/None workspace + add 7 tests for CLI s…
d1782ac 
…ession fallback

Two changes on top of the CLI session fallback from PR #204:

1. Guard against empty/None workspace in the CLI session fallback path:
   cli_meta.get('workspace', '') could return an empty string for sessions
   created before workspace tracking was added (or with workspace=None).
   Path('') resolves to CWD which is silently wrong. Now returns 400 instead.

2. Add 7 unit tests in test_cli_session_filebrowser.py:
   - CLI fallback is called when WebUI session not found (KeyError path)
   - WebUI sessions go through direct path (no CLI fallback)
   - Session not found in CLI sessions returns 404
   - CLI session with missing workspace key returns 400 (not KeyError crash)
   - CLI session with empty workspace returns 400 (not CWD traversal)
   - CLI session with None workspace returns 400 (not TypeError)
   - Missing session_id returns error immediately
@nesquena-hermes

nesquena-hermes commented Apr 10, 2026

Copy link
Copy Markdown
Collaborator

Rebased onto current master (post #196, #197, #198, #199, #201 merges). The fix is correct and the approach matches the existing fallback pattern used in /api/session GET and /api/sessions list.

Added tests covering:

  • Missing session_id param → 400
  • Unknown session_id (not in WebUI or CLI) → 404
  • Structural verification that the CLI fallback (get_cli_sessions()) is present and has the correct shape

566 tests passing. Ready to merge.

@nesquena-hermes
nesquena-hermes merged commit be92e59 into nesquena:master Apr 10, 2026
@nesquena-hermes nesquena-hermes mentioned this pull request Apr 10, 2026
Merged
iRonin added a commit to iRonin/hermes-webui that referenced this pull request Apr 10, 2026
@iRonin
fix: CLI session chat support + mobile workspace panel close
de5a068 
1. _handle_chat_start: Import CLI sessions on-the-fly when sending a
   message to a CLI session from the WebUI. Previously returned 404
   because only WebUI in-memory sessions were checked.

2. Mobile workspace panel: Added closeMobileFiles() function and wired
   it into closeMobileSidebar() so tapping the overlay dismisses both
   the sidebar AND the workspace panel.

Fixes nesquena#204 (CLI session chat), fixes mobile workspace panel close.
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
@iRonin
fix: support CLI sessions in /api/list file browser (nesquena#204)
7333081 
* feat: optional HTTPS/TLS support via cert and key env vars

Add optional HTTPS support controlled by two env vars:
  HERMES_WEBUI_TLS_CERT=/path/to/cert.pem
  HERMES_WEBUI_TLS_KEY=/path/to/key.pem

- Wraps server socket with ssl.SSLContext (min TLSv1.2)
- Dynamic scheme detection for startup messages (http:// vs https://)
- Graceful fallback to HTTP if cert loading fails — server never crashes
  due to bad TLS config, just prints a warning and continues
- Auth cookie Secure flag already set when HTTPS is detected via getpeercert
- 6 end-to-end tests: config flags, HTTPS handshake, HTTP still works,
  fallback on bad paths

Addresses nesquena#191 (HTTPS support issue).

* fix: use current branch upstream for update checks, not repo default branch

The update checker in api/updates.py always compared HEAD against
origin/master (or origin/main), which produced false 'N updates
available' alerts when the user is on a feature branch and master has
moved forward with unrelated commits.

Now uses git rev-parse --abbrev-ref @{upstream} to get the current
branch's tracking branch for both the behind-count check and the
apply-update pull command. Falls back to the default branch if no
upstream is set (brand-new local branch with no tracking config).

Fixes nesquena#200.

* fix: support CLI sessions in /api/list file browser

_handle_list_dir() only checked WebUI in-memory sessions, returning
'Session not found' for CLI sessions imported from the agent's state.db.
Now falls back to get_cli_sessions() to find the workspace path for
CLI sessions that aren't loaded in WebUI memory.

Fixes: workspace pane showing empty for CLI sessions.
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
@iRonin
fix: support CLI sessions in /api/list file browser (nesquena#204)
04b3d31 
* feat: optional HTTPS/TLS support via cert and key env vars

Add optional HTTPS support controlled by two env vars:
  HERMES_WEBUI_TLS_CERT=/path/to/cert.pem
  HERMES_WEBUI_TLS_KEY=/path/to/key.pem

- Wraps server socket with ssl.SSLContext (min TLSv1.2)
- Dynamic scheme detection for startup messages (http:// vs https://)
- Graceful fallback to HTTP if cert loading fails — server never crashes
  due to bad TLS config, just prints a warning and continues
- Auth cookie Secure flag already set when HTTPS is detected via getpeercert
- 6 end-to-end tests: config flags, HTTPS handshake, HTTP still works,
  fallback on bad paths

Addresses nesquena#191 (HTTPS support issue).

* fix: use current branch upstream for update checks, not repo default branch

The update checker in api/updates.py always compared HEAD against
origin/master (or origin/main), which produced false 'N updates
available' alerts when the user is on a feature branch and master has
moved forward with unrelated commits.

Now uses git rev-parse --abbrev-ref @{upstream} to get the current
branch's tracking branch for both the behind-count check and the
apply-update pull command. Falls back to the default branch if no
upstream is set (brand-new local branch with no tracking config).

Fixes nesquena#200.

* fix: support CLI sessions in /api/list file browser

_handle_list_dir() only checked WebUI in-memory sessions, returning
'Session not found' for CLI sessions imported from the agent's state.db.
Now falls back to get_cli_sessions() to find the workspace path for
CLI sessions that aren't loaded in WebUI memory.

Fixes: workspace pane showing empty for CLI sessions.
@uzairansaruzi uzairansaruzi mentioned this pull request Aug 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iRonin @nesquena-hermes @nesquena