[Timmy] Verify Config Structure Validation at Startup (#116) #129
Assignees
Rockachopa
Timmy
allegro
antigravity
bezalel
claude
claw-code
codex-agent
ezra
gemini
google
grok
groq
hermes
kimi
manus
perplexity
sonnet
Clear assignees
Rockachopa
Timmy
allegro
antigravity
bezalel
claude
claw-code
codex-agent
ezra
gemini
google
grok
groq
hermes
kimi
manus
perplexity
sonnet
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Timmy_Foundation/hermes-agent#129
Reference in New Issue
Block a user
Delete Branch "timmy/issue-116-config-validation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Verifies that the config structure validation added in commit
dce5f51c(#5426) correctly handles all expected scenarios at startup.What Was Verified
The
validate_config_structure()function inhermes_cli/config.pycorrectly:Test Script Added
scripts/test_config_validation.py- 25 comprehensive tests using subprocess isolation to test real YAML file loading throughload_config()->validate_config_structure()pipeline. All 25/25 pass.Closes #116
OPENAI_BASE_URL was written to .env AND config.yaml, creating a dual-source confusion. Users (especially Docker) would see the URL in .env and assume that's where all config lives, then wonder why LLM_MODEL in .env didn't work. Changes: - Remove all 27 save_env_value("OPENAI_BASE_URL", ...) calls across main.py, setup.py, and tools_config.py - Remove OPENAI_BASE_URL env var reading from runtime_provider.py, cli.py, models.py, and gateway/run.py - Remove LLM_MODEL/HERMES_MODEL env var reading from gateway/run.py and auxiliary_client.py — config.yaml model.default is authoritative - Vision base URL now saved to config.yaml auxiliary.vision.base_url (both setup wizard and tools_config paths) - Tests updated to set config values instead of env vars Convention enforced: .env is for SECRETS only (API keys). All other configuration (model names, base URLs, provider selection) lives exclusively in config.yaml.The delivery target parser uses split(':', 1) which only splits on the first colon. For the documented format platform:chat_id:thread_id (e.g. 'telegram:-1001234567890:17585'), thread_id gets munged into chat_id and is never extracted. Fix: split(':', 2) to correctly extract all three parts. Also fix to_string() to include thread_id for proper round-tripping. The downstream plumbing in _deliver_to_platform() already handles thread_id correctly (line 292-293) — it just never received a value.* fix: root-level provider in config.yaml no longer overrides model.provider load_cli_config() had a priority inversion: a stale root-level 'provider' key in config.yaml would OVERRIDE the canonical 'model.provider' set by 'hermes model'. The gateway reads model.provider directly from YAML and worked correctly, but 'hermes chat -q' and the interactive CLI went through the merge logic and picked up the stale root-level key. Fix: root-level provider/base_url are now only used as a fallback when model.provider/model.base_url is not set (never as an override). Also added _normalize_root_model_keys() to config.py load_config() and save_config() — migrates root-level provider/base_url into the model section and removes the root-level keys permanently. Reported by (≧▽≦) in Discord: opencode-go provider persisted as a root-level key and overrode the correct model.provider=openrouter, causing 401 errors. * fix(security): redact secrets from execute_code sandbox output The execute_code sandbox stripped env vars with secret-like names from the child process (preventing os.environ access), but scripts could still read secrets from disk (e.g. open('~/.hermes/.env')) and print them to stdout. The raw values entered the model context unredacted. terminal_tool and file_tools already applied redact_sensitive_text() to their output — execute_code was the only tool that skipped this step. Now the same redaction runs on both stdout and stderr after ANSI stripping. Reported via Discord (not filed on GitHub to avoid public disclosure of the reproduction steps).When PyYAML is unavailable or YAML frontmatter is malformed, the fallback parser may return metadata as a string instead of a dict. This causes AttributeError when calling .get("hermes") on the string. Added explicit type checks to handle cases where metadata or hermes fields are not dicts, preventing the crash. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>OpenAI's newer models (GPT-5, Codex) give stronger instruction-following weight to the 'developer' role vs 'system'. Swap the role at the API boundary in _build_api_kwargs() for the chat_completions path so internal message representation stays consistent ('system' everywhere). Applies regardless of provider — OpenRouter, Nous portal, direct, etc. The codex_responses path (direct OpenAI) uses 'instructions' instead of message roles, so it's unaffected. DEVELOPER_ROLE_MODELS constant in prompt_builder.py defines the matching model name substrings: ('gpt-5', 'codex').By default, Hermes always threads replies to channel messages. Teams that prefer direct channel replies had no way to opt out without patching the source. Add a reply_in_thread option (default: true) to the Slack platform extra config: platforms: slack: extra: reply_in_thread: false When false, _resolve_thread_ts() returns None for top-level channel messages, so replies go directly to the channel. Messages already inside an existing thread are still replied in-thread to preserve conversation context. Default is true for full backward compatibility.ACP clients pass MCP server definitions in session/new, load_session, resume_session, and fork_session. Previously these were accepted but silently ignored — the agent never connected to them. This wires the mcp_servers parameter into the existing MCP registration pipeline (tools/mcp_tool.py) so client-provided servers are connected, their tools discovered, and the agent's tool surface refreshed before the first prompt. Changes: tools/mcp_tool.py: - Extract sanitize_mcp_name_component() to replace all non-[A-Za-z0-9_] characters (fixes crash when server names contain / or other chars that violate provider tool-name validation rules) - Use it in _convert_mcp_schema, _sync_mcp_toolsets, _build_utility_schemas - Extract register_mcp_servers(servers: dict) as a public API that takes an explicit {name: config} map. discover_mcp_tools() becomes a thin wrapper that loads config.yaml and calls register_mcp_servers() acp_adapter/server.py: - Add _register_session_mcp_servers() which converts ACP McpServerStdio / McpServerHttp / McpServerSse objects to Hermes MCP config dicts, registers them via asyncio.to_thread (avoids blocking the ACP event loop), then rebuilds agent.tools, valid_tool_names, and invalidates the cached system prompt - Call it from new_session, load_session, resume_session, fork_session Tested with Eden (theproxycompany.com) as ACP client — 5 MCP servers (HTTP + stdio) registered successfully, 110 tools available to the agent.- Detect if origin points to a fork (not NousResearch/hermes-agent) - Show warning when updating from a fork: origin URL - After pulling from origin/main on a fork: - Prompt to add upstream remote if not present - Respect ~/.hermes/.skip_upstream_prompt to avoid repeated prompts - Compare origin/main with upstream/main - If origin has commits not on upstream, skip (don't trample user's work) - If upstream is ahead, pull from upstream and try to sync fork - Use --force-with-lease for safe fork syncing Non-main branches are unaffected - they just pull from origin/{branch}. Co-authored-by: Avery <avery@hermes-agent.ai>When config.yaml has 'mcp_servers:' with no value, YAML parses it as None. dict.get('mcp_servers', {}) only returns the default when the key is absent, not when it's explicitly None. Use 'or {}' pattern to handle both cases, matching the other two assignment sites in the same file.Two fixes for Discord exec approval: 1. Register /approve and /deny as native Discord slash commands so they appear in Discord's command picker (autocomplete). Previously they were only handled as text commands, so users saw 'no commands found' when typing /approve. 2. Wire up the existing ExecApprovalView button UI (was dead code): - ExecApprovalView now calls resolve_gateway_approval() to actually unblock the waiting agent thread when a button is clicked - Gateway's _approval_notify_sync() detects adapters with send_exec_approval() and routes through the button UI - Added 'Allow Session' button for parity with /approve session - send_exec_approval() now accepts session_key and metadata for thread support - Graceful fallback to text-based /approve prompt if button send fails Also updates test mocks to include grey/secondary ButtonStyle and purple Color (used by new button styles).Three fixes for memory+profile isolation bugs: 1. memory_tool.py: Replace module-level MEMORY_DIR constant with get_memory_dir() function that calls get_hermes_home() dynamically. The old constant was cached at import time and could go stale if HERMES_HOME changed after import. Internal MemoryStore methods now call get_memory_dir() directly. MEMORY_DIR kept as backward-compat alias. 2. profiles.py: profile create --clone now copies MEMORY.md and USER.md from the source profile. These curated memory files are part of the agent's identity (same as SOUL.md) and should carry over on clone. 3. holographic plugin: initialize() now expands $HERMES_HOME and ${HERMES_HOME} in the db_path config value, so users can write 'db_path: $HERMES_HOME/memory_store.db' and it resolves to the active profile directory, not the default home. Tests updated to mock get_memory_dir() alongside the legacy MEMORY_DIR.Windows (CRLF) and old Mac (CR) line endings are normalised to LF before the 5-line collapse threshold is checked in handle_paste. Without this, markdown copied from Windows sources contains \r\n but the line counter (pasted_text.count('\n')) still works — however buf.insert_text() leaves bare \r characters in the buffer which some terminals render by moving the cursor to the start of the line, making multi-line pastes appear as a single overwritten line.Reads config.extra['group_topics'] to bind skills to specific thread_ids in supergroup/forum chats. Mirrors the dm_topics skill injection pattern but for group chat_type. Enables per-topic skill auto-loading in Falcon HQ. Config format: platforms.telegram.extra.group_topics: - chat_id: -1003853746818 topics: - name: FalconConnect thread_id: 5 skill: falconconnect-architectureThe config key skills.external_dirs and core resolution (get_all_skills_dirs, get_external_skills_dirs in agent/skill_utils.py) already existed but several code paths still only scanned SKILLS_DIR. Now external dirs are respected everywhere: - skills_categories(): scan all dirs for category discovery - _get_category_from_path(): resolve categories against any skills root - skill_manager_tool._find_skill(): search all dirs for edit/patch/delete - credential_files.get_skills_directory_mount(): mount all dirs into Docker/Singularity containers (external dirs at external_skills/<idx>) - credential_files.iter_skills_files(): list files from all dirs for Modal/Daytona upload - tools/environments/ssh.py: rsync all skill dirs to remote hosts - gateway _check_unavailable_skill(): check disabled skills across all dirs Usage in config.yaml: skills: external_dirs: - ~/repos/agent-skills/hermes - /shared/team-skillsTwo pre-existing issues causing test_file_read_guards timeouts on CI: 1. agent/redact.py: _ENV_ASSIGN_RE used unbounded [A-Z_]* with IGNORECASE, matching any letter/underscore to end-of-string at each position → O(n²) backtracking on 100K+ char inputs. Bounded to {0,50} since env var names are never that long. 2. tools/file_tools.py: redact_sensitive_text() ran BEFORE the character-count guard, so oversized content (that would be rejected anyway) went through the expensive regex first. Reordered to check size limit before redaction.Add MiniMax as a fifth TTS provider alongside Edge TTS, ElevenLabs, OpenAI, and NeuTTS. Supports speech-2.8-hd (recommended default) and speech-2.8-turbo models via the MiniMax T2A HTTP API. Changes: - Add _generate_minimax_tts() with hex-encoded audio decoding - Add MiniMax to provider dispatch, requirements check, and Telegram Opus compatibility handling - Add MiniMax to interactive setup wizard with API key prompt - Update TTS documentation and config example Configuration: tts: provider: "minimax" minimax: model: "speech-2.8-hd" voice_id: "English_Graceful_Lady" Requires MINIMAX_API_KEY environment variable. API reference: https://platform.minimax.io/docs/api-reference/speech-t2a-httpAdd docker_env option to terminal config — a dict of key-value pairs that get set inside Docker containers via -e flags at both container creation (docker run) and per-command execution (docker exec) time. This complements docker_forward_env (which reads values dynamically from the host process environment). docker_env is useful when Hermes runs as a systemd service without access to the user's shell environment — e.g. setting SSH_AUTH_SOCK or GNUPGHOME to known stable paths for SSH/GPG agent socket forwarding. Precedence: docker_env provides baseline values; docker_forward_env overrides for the same key. Config example: terminal: docker_env: SSH_AUTH_SOCK: /run/user/1000/ssh-agent.sock GNUPGHOME: /root/.gnupg docker_volumes: - /run/user/1000/ssh-agent.sock:/run/user/1000/ssh-agent.sock - /run/user/1000/gnupg/S.gpg-agent:/root/.gnupg/S.gpg-agent- Browse: POST /api/v1/browse → GET /api/v1/fs/{ls,tree,stat} - Read: POST /api/v1/read[/abstract] → GET /api/v1/content/{read,abstract,overview} - System prompt: result.get('children') → len(result) (API returns list) - Content: result.get('content') → result is a plain string - Browse: result['entries'] → result is the list; is_dir → isDir (camelCase) - Browse: add rel_path and abstract fields to entry output Based on PR #4742 by catbusconductor. Auth header changes dropped (already on main via #4825).LLMs frequently return numbers as strings ("42" instead of 42) and booleans as strings ("true" instead of true). This causes silent failures with MCP tools and any tool with strictly-typed parameters. Added coerce_tool_args() in model_tools.py that runs before every tool dispatch. For each argument, it checks the tool registry schema and attempts safe coercion: - "42" → 42 when schema says "type": "integer" - "3.14" → 3.14 when schema says "type": "number" - "true"/"false" → True/False when schema says "type": "boolean" - Union types tried in order - Original values preserved when coercion fails or is not applicable Inspired by Block/goose tool argument coercion system.* feat: coerce tool call arguments to match JSON Schema types LLMs frequently return numbers as strings ("42" instead of 42) and booleans as strings ("true" instead of true). This causes silent failures with MCP tools and any tool with strictly-typed parameters. Added coerce_tool_args() in model_tools.py that runs before every tool dispatch. For each argument, it checks the tool registry schema and attempts safe coercion: - "42" → 42 when schema says "type": "integer" - "3.14" → 3.14 when schema says "type": "number" - "true"/"false" → True/False when schema says "type": "boolean" - Union types tried in order - Original values preserved when coercion fails or is not applicable Inspired by Block/goose tool argument coercion system. * fix: accept reasoning-only responses without retries — set content to "(empty)" Previously, when a model returned reasoning/thinking but no visible content, we entered a 120-line retry/classify/compress/salvage cascade that wasted 3+ API calls trying to "fix" the response. The model was done thinking — retrying with the same input just burned money. Now reasoning-only responses are accepted immediately: - Reasoning stays in the `reasoning` field (semantically correct) - Content set to "(empty)" — valid non-empty string every provider accepts - No retries, no compression triggers, no salvage logic - Session history contains "(empty)" not "" — prevents #2128 session poisoning where empty assistant content caused prefill rejections Removes ~120 lines, adds ~15. Saves 2-3 API calls per reasoning-only response. Fixes #2128.Add POST /v1/runs to start async agent runs and GET /v1/runs/{run_id}/events for SSE streaming of typed lifecycle events (tool.started, tool.completed, message.delta, reasoning.available, run.completed, run.failed). Changes the internal tool_progress_callback signature from positional (tool_name, preview, args) to event-type-first (event_type, tool_name, preview, args, **kwargs). Existing consumers filter on event_type and remain backward-compatible. Adds concurrency limit (_MAX_CONCURRENT_RUNS=10) and orphaned run sweep. Fixes logic inversion in cli.py _on_tool_progress where the original PR would have displayed internal tools instead of non-internal ones. Co-authored-by: Mibayy <mibayy@users.noreply.github.com>Add optional 'expression' parameter to browser_console that evaluates JavaScript in the page context (like DevTools console). Returns structured results with auto-JSON parsing. No new tool — extends the existing browser_console schema with ~20 tokens of overhead instead of adding a 12th browser tool. Both backends supported: - Browserbase: uses agent-browser 'eval' command via CDP - Camofox: uses /tabs/{tab_id}/eval endpoint with graceful degradation E2E verified: string eval, number eval, structured JSON, DOM manipulation, error handling, and original console-output mode all working.Implement tools/mcp_oauth.py — the OAuth adapter that mcp_tool.py's existing auth: oauth hook has been waiting for. Components: - HermesTokenStorage: persists tokens + client registration to HERMES_HOME/mcp-tokens/<server>.json with 0o600 permissions - Callback handler factory: per-flow isolated HTTP handlers (safe for concurrent OAuth flows across multiple MCP servers) - OAuthClientProvider integration: wraps the MCP SDK's httpx.Auth subclass which handles discovery, DCR, PKCE, token exchange, refresh, and step-up auth (403 insufficient_scope) automatically - Non-interactive detection: warns when gateway/cron environments try to OAuth without cached tokens - Pre-registered client support: injects client_id/secret from config for servers that don't support Dynamic Client Registration (e.g. Slack) - Path traversal protection on server names - remove_oauth_tokens() for cleanup Config format: mcp_servers: sentry: url: 'https://mcp.sentry.dev/mcp' auth: oauth oauth: # all optional client_id: '...' # skip DCR client_secret: '...' # confidential client scope: 'read write' # server-provided by default Also passes oauth config dict through from mcp_tool.py (was passing only server_name and url before). E2E verified: full OAuth flow (401 → discovery → DCR → authorize → token exchange → authenticated request → tokens persisted) against local test servers. 23 unit tests + 186 MCP suite tests pass.Cron agents were burning iterations trying to use send_message (which is disabled via messaging toolset) because their prompts said things like 'send the report to Telegram'. The scheduler handles delivery automatically via the deliver setting, but nothing told the agent that. Add a delivery guidance hint to _build_job_prompt alongside the existing [SILENT] hint: tells agents their final response is auto-delivered and they should NOT use send_message. Before: only [SILENT] suppression hint After: delivery guidance ('do NOT use send_message') + [SILENT] hintEnable Hermes tool execution through the copilot-acp adapter by: - Passing tool schemas and tool_choice into the ACP prompt text - Instructing ACP backend to emit <tool_call>{...}</tool_call> blocks - Parsing XML tool-call blocks and bare JSON fallback back into Hermes-compatible SimpleNamespace tool call objects - Setting finish_reason='tool_calls' when tool calls are extracted - Cleaning tool-call markup from response text Fix duplicate tool call extraction when both XML block and bare JSON regexes matched the same content (XML blocks now take precedence). Cherry-picked from PR #4536 by MestreY0d4-Uninter. Stripped heuristic fallback system (auto-synthesized tool calls from prose) and Portuguese-language patterns — tool execution should be model-decided, not heuristic-guessed.🏷️ Automated Triage Check
Timestamp: 2026-04-06T14:30:13.900266
Agent: Allegro Heartbeat
This issue has been identified as needing triage:
Checklist
Context
Automated triage from Allegro 15-minute heartbeat
Cross-Audit Triage Review
Decision: CLOSED (stale, merge conflicts, scope creep)
This PR is titled "Verify Config Structure Validation at Startup" but touches 30 files (+4550/-170) across the entire codebase including Dockerfile, CI workflows, ACP adapter, credential pool, memory manager, and many other unrelated modules.
Key issues:
Recommendation: Rebase onto current main, isolate only the config validation changes into a focused PR, and resubmit.
— Automated cross-audit triage
Pull request closed