fix(skill_manager): use patchable SKILLS_DIR in _find_skill - #5268
Closed
iRonin wants to merge 44 commits into
Closed
fix(skill_manager): use patchable SKILLS_DIR in _find_skill#5268iRonin wants to merge 44 commits into
iRonin wants to merge 44 commits into
Conversation
Adds two features to the Hermes CLI: Ctrl+G — External Editor: - Opens current input in $VISUAL / $EDITOR / VS Code / Cursor / vi - Smart paste detection: if input contains a collapsed paste reference [Pasted text #N → path], opens that file directly for editing - Uses run_in_terminal() for clean TUI suspend/resume - Updates input buffer and paste line count on editor close /keys (/shortcuts) — Keyboard Shortcuts Display: - Categorized list of all keybindings (Input, Session, Drafting, Voice) - Reads voice key from config for accurate display - Registered in CommandDef with tab completion
…icators Adds a Claude Code-style input stash to the Hermes CLI: - Ctrl+S stashes current input (text + attached images) and clears the field - Ctrl+S on empty input pops the stash back - Stashed input auto-restores after the agent finishes responding - Placeholder shows stash preview when idle, hint when agent is running - Status bar shows a pinned indicator when a stash is active - Uses (text, [images]) tuple so dragged/pasted images are preserved Alternative to NousResearch#4259 with additional features: auto-restore after response (the key UX from Claude Code), image stashing, placeholder preview, status bar indicator, and proper buf.reset() cleanup. Closes NousResearch#4255
Input starting with / is only routed to the command handler when the
first word matches a known command (via resolve_command). Bare paths
like /Users/ironin/file.md:45-46 now pass through as regular input
to the agent instead of triggering 'Unknown command'.
Fixes both the process_loop routing and the handle_enter interrupt
bypass — both had the same startswith('/') assumption.
Alt+Enter now queues the current input as a follow-up to be sent after the agent finishes responding, instead of inserting a newline. - Alt+Enter → puts message into _pending_input (non-interrupting) - Enter (agent running) → still interrupts via _interrupt_queue - _followup_queue list mirrors pending items for display - Status bar shows 📬 N when follow-ups are queued - Placeholder hints update: shows queue depth while agent runs, and persists after it finishes until queue drains - Ctrl+J remains the newline key for multi-line input Closes: the need for Shift+Enter queue (terminals can't distinguish Shift+Enter from Enter; Alt+Enter is the reliable alternative)
Alt+Up pops the most recently queued follow-up (LIFO) from _followup_queue, appends it to the current input with a newline--- separator, and marks it cancelled so process_loop skips it. Repeated Alt+Up recalls one at a time until queue is empty. _cancelled_followups set is checked in process_loop and discarded on match to avoid sending the recalled message twice.
hermes -c "session name" -m anthropic/claude-sonnet-4-6 now works. Previously -m was only on 'hermes chat', so the shorthand root-level -c flag couldn't be combined with a model override. Also stop stomping args.model/provider with None in the root→chat passthrough — the values from the root parser are now preserved.
Addresses review feedback from britrik (NousResearch#4788): - Replace text-based cancellation with UUID tags — identical messages queued twice no longer cancel each other incorrectly - Wrap Alt+Enter payloads as {_followup_tag, payload} dicts so process_loop can identify followup items by ID, not content - Fix phantom _followup_queue pops: display sync now only happens for tagged (Alt+Enter) items, not regular Enter messages - _cancelled_followups stores UUIDs (bounded, auto-discarded on match) Note: the image-payload cancel check was already correct in the original — both sides extracted text via payload[0] — but UUID tagging makes the intent unambiguous regardless of payload shape.
…dicator Sets the terminal title via OSC 0 escape sequence (\x1b]0;...\x07): ⚕ Hermes — session name (named session, idle) ⚕ Hermes ⏳ (agent thinking) ⚕ Hermes (unnamed session) Symbol comes from the active skin's response_label (⚕ default, ⚔ Ares, etc.) so it adapts to the current theme. Updated at: - run() startup - _preload_resumed_session() when a titled session is resumed - /title command when a title is set or committed from pending - process_loop when agent starts (thinking=True) and finishes Skipped when stdout is not a TTY, TERM=dumb, or NO_COLOR is set.
Three input UX improvements: 1. ESC ESC — clear input buffer (and attached images) Pressing ESC twice quickly discards the current draft without conflicting with Alt key sequences (escape+enter, escape+up, etc.) 2. Ctrl+P — peek collapsed paste content inline When input contains a [Pasted text #N → path] reference, prints the first 20 lines in the terminal so the user can verify content without opening an editor (Ctrl+G). Falls back to previewing the current input text when no paste reference is present. 3. \r\n normalisation in handle_paste Windows-style (CRLF) and old Mac-style (CR) line endings are normalised to LF before the 5-line collapse threshold is checked. Prevents markdown pasted from Windows sources being treated as single-line and bypassing the file-reference collapse.
Ctrl+P is now context-aware:
- paste ref in input → peek first 20 lines of paste file inline
- text in input → preview first 20 lines of current input
- empty input → full conversation pager (newest first, via less)
New show_history_full() method:
- Reverses conversation order so most recent message is at the top
- No truncation — full text of every user and assistant turn
- Tool call names listed inline on the header line
- Strips REASONING_SCRATCHPAD blocks
- Pipes through 'less -R --no-init --quit-if-one-screen'
(falls back to plain print if less is unavailable)
- Header shows message count and keyboard hints (q, /)
New /history full (aliases: f, all):
- Calls show_history_full() from the slash command interface
- /history (no arg) still calls the existing show_history()
- Drop 'Hermes' and session name from tab title — symbol only - Use OSC 1 (tab/icon title) + OSC 2 (window title) instead of OSC 0 so iTerm2 does not append the Python process name to the tab label - Thinking indicator: ⚕ ⏳ (was ⚕ Hermes ⏳) - Idle: ⚕ (was ⚕ Hermes / ⚕ Hermes — session)
… proxy Inside the TUI, sys.stdout is patched by prompt_toolkit's patch_stdout. OSC escape sequences written to StdoutProxy are buffered or discarded and never reach the terminal emulator — so /title didn't update the tab and Python still appeared as the process name. Fix: use sys.__stdout__ (the pre-patch original) and write via os.write() directly to the file descriptor, bypassing the proxy entirely.
Users on tmux/screen, or whose iTerm2 profile appends the job name
(Python) to the tab title, can now disable OSC title sequences:
display:
terminal_title: false
Default: true (enabled).
…logins /browser connect profile — Default profile (cookies/logins intact) /browser connect profile 'Profile 1' — specific Chrome profile /browser connect ws://... — custom CDP URL (unchanged) Passes --user-data-dir and --profile-directory to Chrome so existing sessions (X, LinkedIn, etc.) are available. Warns that Chrome must be fully quit first (Cmd+Q) since Chrome is single-instance per profile.
Complete rewrite of browser command handling:
/browser connect — auto-launches Chrome with ~/.hermes/chrome-profile
(or browser.hermes_profile_dir from config)
auto-detects if already running, no URL needed
/browser connect setup — first-time setup: creates profile dir, opens Chrome
so user can log in, then close and run connect
/browser connect <url> — explicit CDP URL (unchanged)
Config options:
browser.hermes_profile_dir: ~/.hermes/chrome-profile
browser.cdp_port: 9222
Fixes BROWSER_CDP_URL being set to literal words ('profile', 'hermes')
instead of the actual http://localhost:PORT endpoint.
Adds _chrome_candidates(), _ensure_chrome_debug() helpers.
…ering race os.write() to fd 1 races with prompt_toolkit's own rendering writes, causing ESC to appear as '?' and the OSC sequence to leak as literal text (e.g. '?]0;⚕ Hermes' visible in the terminal output). Fix: when inside the TUI, write via get_app().output.write_raw() which is synchronised with the render loop. Falls back to direct fd write when outside the TUI (startup, single-query mode).
…mode) Two distinct queues with dedicated icons and recall shortcuts: 📬 Follow-up queue (Alt+Enter, always non-interrupting) - New: independent follow-up task after current response - Recall: Alt+Up (LIFO) - Status bar: 📬 N - UUID-tagged, cancellable 🎯 Steering queue (Enter during agent run, busy_input_mode=queue) - Contextual guidance for current/upcoming work - Recall: Alt+Down (LIFO) - Status bar: 🎯 N - UUID-tagged, cancellable Placeholder hints adapt to busy_input_mode: queue mode: 'Enter to steer (🎯) · Alt+Enter to follow-up (📬)' interrupt mode: 'Enter to interrupt · Alt+Enter to queue follow-up (📬)' Idle placeholder shows both counts with their recall shortcuts.
Agent-running placeholder now shows: 📬 2 (Alt+↑ to recall) · 🎯 1 (Alt+↓ to recall) instead of just the counts.
The startup call to _update_terminal_title() fired before app.run(), so get_app() raised RuntimeError and the os.write() fallback ran — but iTerm2 then reset the tab title when prompt_toolkit took over the terminal, leaving the tab showing 'hermes (Python)' with no ⚕ symbol. Fix: remove the premature pre-app call and instead schedule _update_terminal_title() via app.call_from_executor() at the start of process_loop. This runs in the event loop after the TUI is live, so get_app() returns the running app and write_raw() reaches the terminal at the right moment.
get_app().output.write_raw() and os.write(sys.__stdout__.fileno()) both fail to reliably reach the terminal because prompt_toolkit's output buffers and patch_stdout intercept or defer the write. Fix: open the controlling terminal device via os.ctermid() (returns '/dev/tty' on macOS/Linux) with O_WRONLY|O_NOCTTY and write directly. This bypasses Python's I/O, prompt_toolkit's buffers, patch_stdout's StdoutProxy, and any stdout redirections — bytes go straight to the TTY the user is looking at, from any thread, at any time.
Debug test showed Test 5 (OSC 0 + ST terminator) is what iTerm2 accepts for tab title updates. Previous code used OSC 1+2 with BEL (\x07). Changes: - OSC 0 instead of separate OSC 1 + OSC 2 - ST terminator (\x1b\\) instead of BEL (\x07) - Write via sys.__stdout__ first (simpler), ctermid as fallback
Tab title format: ⚕ My Session (when /title is set) ⚕ ⏳ (when agent is thinking) ⚕ (no title set)
Multiline user messages are always displayed truncated (first line + '+N lines'). This adds a config option and runtime toggle. - display.show_full_user_message: false (default) in config.yaml - Ctrl+O toggles at runtime with '↕ Full user message display: ON/OFF' feedback — no restart required - Status bar shows '↕ full msg' indicator when active - To enable permanently: hermes config set display.show_full_user_message true
Lets Open WebUI (or any OpenAI-compatible frontend) select the underlying LLM via the model field in the chat completions request. When model_override is set and is not 'hermes-agent', it is used instead of the value from config.yaml. GET /v1/models now returns hermes-agent plus the per-provider model list.
browser_tool: read browser.cdp_url from config.yaml as a persistent fallback for BROWSER_CDP_URL, so /browser connect is not needed every session when cdp_url is set in config. cli: when thinking and a session title exists, append ⏳ to the title rather than replacing it — was: '⚕ ⏳', now: '⚕ My Title ⏳'.
All four response panel sites (streaming box, TTS box, background
task panel, main Rich panel) now append '— {title}' to the label
when a session title exists:
╭─ ⚕ Hermes — My Session Title ────────╮
Also: tab title appends ⏳ instead of replacing the session title
when thinking (was: '⚕ ⏳', now: '⚕ My Title ⏳').
delegate_tool: max_concurrent_children now reads from
delegation.max_concurrent_children in config.yaml (default 6).
Auto-restore after agent response now checks buf.text.strip() first. If the user started typing while the agent was responding, the stash is left intact with a 'Ctrl+S to pop' reminder instead of clobbering their input.
/resume with no argument now calls show_sessions_full() which fetches up to 200 sessions and pipes them through less (same mechanism as Ctrl+P history pager) — scrollable, searchable with '/'. Replaces the hardcoded 10-session inline table.
/resume with no arg now opens a mini prompt_toolkit Application: - type to fuzzy-filter by label/preview/id in real-time - ↑↓ navigate, Enter to select and auto-resume, Esc/q/Ctrl+C cancel - sessions with no title show first user message as label instead of '—' - up to 200 sessions listed - falls back to less pager on non-interactive terminals
display.steering_dispatch: all_at_once | one_by_one (default) display.followup_dispatch: one_by_one (default) | all_at_once all_at_once: items held in the queue list only until the current agent turn completes, then drained and joined with \n---\n into a single combined message for the next turn. one_by_one: existing behaviour — each queued item goes straight into _pending_input and triggers its own agent turn.
- Rename 'Label' column header → 'Title' everywhere in the picker and fallback pager; rename _label() helper → _title() - list_sessions_rich: ORDER BY last_active DESC instead of started_at DESC — most recently used sessions appear first - Only CLI sessions shown (source=cli, excludes tool/gateway/cron)
Off by default — only CLI sessions shown. Set to true to include gateway sessions (Telegram, Discord, etc.) in the /resume picker. Tool sessions always excluded.
Lists sessions from the shared DB. Query params:
source — filter by source (cli, telegram, discord, etc); omit for all
limit — max results (default 50, max 200)
offset — pagination offset (default 0)
Returns: {object: list, data: [{id, title, preview, last_active,
source, message_count}], count: N}
Tool-spawned sessions always excluded.
Also fixes pre-existing syntax error in _handle_models (garbled auth
check line restored to self._check_auth(request)).
Live overlay showing running/completed subagents with progress. Ctrl+X toggles panel, ↑↓ navigates rows, K interrupts selected agent. Status bar shows 🔀 N badge when N subagents are running. three-file change: - hermes_cli/subagent_panel.py: SubagentRecord dataclass + render_panel() - cli.py: panel state, Ctrl+X keybinding, status badge, TUI widget - tools/delegate_tool.py: panel record lifecycle (spawn/progress/complete)
- Footer dashes: was W-2 (way too wide), now W-19 to match suffix length
- Header dashes: was W-len(title)-14, now W-len(title)-12
- goal_w: was W-12 (too wide), now W-20 (accounts for icon+index+elapsed)
- elapsed field: padded to 9 chars ('0:00 done' max) so row stays fixed-width
- ⚡ (2-wide emoji) replaced with ~ (1-wide) so column math holds
- Tool emoji replaced with ASCII symbols for same reason
- Status bar: '🔀 N Ctrl+X' when closed, '🔀 N ▲' when open
_find_skill() was calling get_all_skills_dirs() which always returns the real ~/.hermes/skills/ as the first entry, ignoring any unittest.mock.patch on the module-level SKILLS_DIR constant. Tests patch SKILLS_DIR to a tmp_path, so _create_skill wrote skills there but _find_skill never found them — causing every subsequent edit/patch/delete/write_file/remove_file test to fail. Fix: build search_dirs by replacing the first entry with the module-level SKILLS_DIR (patchable) and keeping external dirs from config (indices 1+) unchanged. In production the two are identical, so no behaviour change.
Contributor
|
The core fix (5 lines in |
Contributor
Author
|
Superseded by #5317 — rebuilt from clean worktree (origin/main only, single file). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
_find_skill()calledget_all_skills_dirs()which always returns the real~/.hermes/skills/as its first entry — ignoring anyunittest.mock.patchon the module-levelSKILLS_DIRconstant.Tests patch
SKILLS_DIRto atmp_path, so_create_skillwrote skills there but_find_skillnever found them. Every operation that needed to locate an existing skill (edit, patch, delete, write_file, remove_file) silently failed with "skill not found", and the duplicate-check test passed when it shouldn't.Fix
Replace
get_all_skills_dirs()[0]with the module-levelSKILLS_DIR(which is patchable), keeping external skill dirs from config at indices 1+:In production
SKILLS_DIR == get_all_skills_dirs()[0]so behaviour is identical. In tests the patch is respected.Tests
All 51
test_skill_manager_tooltests now pass locally.