Compare commits
6 Commits
feat/sessi
...
pass-sessi
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e80320069b | ||
|
|
f2027b8bff | ||
|
|
a648022137 | ||
|
|
95b1130485 | ||
|
|
3fb8938cd3 | ||
|
|
c5e8166c8b |
70
cli.py
70
cli.py
@@ -1012,6 +1012,10 @@ class HermesCLI:
|
||||
# Configuration - priority: CLI args > env vars > config file
|
||||
# Model can come from: CLI arg, LLM_MODEL env, OPENAI_MODEL env (custom endpoint), or config
|
||||
self.model = model or os.getenv("LLM_MODEL") or os.getenv("OPENAI_MODEL") or CLI_CONFIG["model"]["default"]
|
||||
# Track whether model was explicitly chosen by the user or fell back
|
||||
# to the global default. Provider-specific normalisation may override
|
||||
# the default silently but should warn when overriding an explicit choice.
|
||||
self._model_is_default = not (model or os.getenv("LLM_MODEL") or os.getenv("OPENAI_MODEL"))
|
||||
|
||||
self._explicit_api_key = api_key
|
||||
self._explicit_base_url = base_url
|
||||
@@ -1126,6 +1130,63 @@ class HermesCLI:
|
||||
self._last_invalidate = now
|
||||
self._app.invalidate()
|
||||
|
||||
def _normalize_model_for_provider(self, resolved_provider: str) -> bool:
|
||||
"""Normalize obviously incompatible model/provider pairings.
|
||||
|
||||
When the resolved provider is ``openai-codex``, the Codex Responses API
|
||||
only accepts Codex-compatible model slugs (e.g. ``gpt-5.3-codex``).
|
||||
If the active model is incompatible (e.g. the OpenRouter default
|
||||
``anthropic/claude-opus-4.6``), swap it for the best available Codex
|
||||
model. Also strips provider prefixes the API does not accept
|
||||
(``openai/gpt-5.3-codex`` → ``gpt-5.3-codex``).
|
||||
|
||||
Returns True when the active model was changed.
|
||||
"""
|
||||
if resolved_provider != "openai-codex":
|
||||
return False
|
||||
|
||||
current_model = (self.model or "").strip()
|
||||
current_slug = current_model.split("/")[-1] if current_model else ""
|
||||
|
||||
# Keep explicit Codex models, but strip any provider prefix that the
|
||||
# Codex Responses API does not accept.
|
||||
if current_slug and "codex" in current_slug.lower():
|
||||
if current_slug != current_model:
|
||||
self.model = current_slug
|
||||
if not self._model_is_default:
|
||||
self.console.print(
|
||||
f"[yellow]⚠️ Stripped provider prefix from '{current_model}'; "
|
||||
f"using '{current_slug}' for OpenAI Codex.[/]"
|
||||
)
|
||||
return True
|
||||
return False
|
||||
|
||||
# Model is not Codex-compatible — replace with the best available
|
||||
fallback_model = "gpt-5.3-codex"
|
||||
try:
|
||||
from hermes_cli.codex_models import get_codex_model_ids
|
||||
|
||||
codex_models = get_codex_model_ids(
|
||||
access_token=self.api_key if self.api_key else None,
|
||||
)
|
||||
fallback_model = next(
|
||||
(mid for mid in codex_models if "codex" in mid.lower()),
|
||||
fallback_model,
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if current_model != fallback_model:
|
||||
if not self._model_is_default:
|
||||
self.console.print(
|
||||
f"[yellow]⚠️ Model '{current_model}' is not supported with "
|
||||
f"OpenAI Codex; switching to '{fallback_model}'.[/]"
|
||||
)
|
||||
self.model = fallback_model
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
def _ensure_runtime_credentials(self) -> bool:
|
||||
"""
|
||||
Ensure runtime credentials are resolved before agent use.
|
||||
@@ -1171,8 +1232,13 @@ class HermesCLI:
|
||||
self.api_key = api_key
|
||||
self.base_url = base_url
|
||||
|
||||
# AIAgent/OpenAI client holds auth at init time, so rebuild if key rotated
|
||||
if (credentials_changed or routing_changed) and self.agent is not None:
|
||||
# Normalize model for the resolved provider (e.g. swap non-Codex
|
||||
# models when provider is openai-codex). Fixes #651.
|
||||
model_changed = self._normalize_model_for_provider(resolved_provider)
|
||||
|
||||
# AIAgent/OpenAI client holds auth at init time, so rebuild if key,
|
||||
# routing, or the effective model changed.
|
||||
if (credentials_changed or routing_changed or model_changed) and self.agent is not None:
|
||||
self.agent = None
|
||||
|
||||
return True
|
||||
|
||||
@@ -203,6 +203,10 @@ def cmd_chat(args):
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# --pass-session-id: include session ID in the agent's system prompt
|
||||
if getattr(args, "pass_session_id", False):
|
||||
os.environ["HERMES_PASS_SESSION_ID"] = "1"
|
||||
|
||||
# Import and run the CLI
|
||||
from cli import main as cli_main
|
||||
|
||||
@@ -1303,6 +1307,12 @@ For more help on a command:
|
||||
default=False,
|
||||
help="Run in an isolated git worktree (for parallel agents)"
|
||||
)
|
||||
parser.add_argument(
|
||||
"--pass-session-id",
|
||||
action="store_true",
|
||||
default=False,
|
||||
help="Include the session ID in the agent's system prompt"
|
||||
)
|
||||
|
||||
subparsers = parser.add_subparsers(dest="command", help="Command to run")
|
||||
|
||||
@@ -1357,6 +1367,12 @@ For more help on a command:
|
||||
default=False,
|
||||
help="Run in an isolated git worktree (for parallel agents on the same repo)"
|
||||
)
|
||||
chat_parser.add_argument(
|
||||
"--pass-session-id",
|
||||
action="store_true",
|
||||
default=False,
|
||||
help="Include the session ID in the agent's system prompt"
|
||||
)
|
||||
chat_parser.set_defaults(func=cmd_chat)
|
||||
|
||||
# =========================================================================
|
||||
|
||||
@@ -1409,9 +1409,10 @@ class AIAgent:
|
||||
|
||||
from hermes_time import now as _hermes_now
|
||||
now = _hermes_now()
|
||||
prompt_parts.append(
|
||||
f"Conversation started: {now.strftime('%A, %B %d, %Y %I:%M %p')}"
|
||||
)
|
||||
timestamp_line = f"Conversation started: {now.strftime('%A, %B %d, %Y %I:%M %p')}"
|
||||
if self.session_id and os.getenv("HERMES_PASS_SESSION_ID"):
|
||||
timestamp_line += f"\nSession ID: {self.session_id}"
|
||||
prompt_parts.append(timestamp_line)
|
||||
|
||||
platform_key = (self.platform or "").lower().strip()
|
||||
if platform_key in PLATFORM_HINTS:
|
||||
|
||||
@@ -162,6 +162,128 @@ def test_runtime_resolution_rebuilds_agent_on_routing_change(monkeypatch):
|
||||
assert shell.api_mode == "codex_responses"
|
||||
|
||||
|
||||
def test_codex_provider_replaces_incompatible_default_model(monkeypatch):
|
||||
"""When provider resolves to openai-codex and no model was explicitly
|
||||
chosen, the global config default (e.g. anthropic/claude-opus-4.6) must
|
||||
be replaced with a Codex-compatible model. Fixes #651."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.delenv("LLM_MODEL", raising=False)
|
||||
monkeypatch.delenv("OPENAI_MODEL", raising=False)
|
||||
|
||||
def _runtime_resolve(**kwargs):
|
||||
return {
|
||||
"provider": "openai-codex",
|
||||
"api_mode": "codex_responses",
|
||||
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"api_key": "test-key",
|
||||
"source": "env/config",
|
||||
}
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _runtime_resolve)
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.format_runtime_provider_error", lambda exc: str(exc))
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.codex_models.get_codex_model_ids",
|
||||
lambda access_token=None: ["gpt-5.2-codex", "gpt-5.1-codex-mini"],
|
||||
)
|
||||
|
||||
shell = cli.HermesCLI(compact=True, max_turns=1)
|
||||
|
||||
assert shell._model_is_default is True
|
||||
assert shell._ensure_runtime_credentials() is True
|
||||
assert shell.provider == "openai-codex"
|
||||
assert "anthropic" not in shell.model
|
||||
assert "claude" not in shell.model
|
||||
assert shell.model == "gpt-5.2-codex"
|
||||
|
||||
|
||||
def test_codex_provider_replaces_incompatible_envvar_model(monkeypatch):
|
||||
"""Exact scenario from #651: LLM_MODEL is set to a non-Codex model and
|
||||
provider resolves to openai-codex. The model must be replaced and a
|
||||
warning printed since the user explicitly chose it."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setenv("LLM_MODEL", "claude-opus-4-6")
|
||||
monkeypatch.delenv("OPENAI_MODEL", raising=False)
|
||||
|
||||
def _runtime_resolve(**kwargs):
|
||||
return {
|
||||
"provider": "openai-codex",
|
||||
"api_mode": "codex_responses",
|
||||
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"api_key": "test-key",
|
||||
"source": "env/config",
|
||||
}
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _runtime_resolve)
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.format_runtime_provider_error", lambda exc: str(exc))
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.codex_models.get_codex_model_ids",
|
||||
lambda access_token=None: ["gpt-5.2-codex", "gpt-5.1-codex-mini"],
|
||||
)
|
||||
|
||||
shell = cli.HermesCLI(compact=True, max_turns=1)
|
||||
|
||||
assert shell._model_is_default is False
|
||||
assert shell._ensure_runtime_credentials() is True
|
||||
assert shell.provider == "openai-codex"
|
||||
assert "claude" not in shell.model
|
||||
assert shell.model == "gpt-5.2-codex"
|
||||
|
||||
|
||||
def test_codex_provider_preserves_explicit_codex_model(monkeypatch):
|
||||
"""If the user explicitly passes a Codex-compatible model, it must be
|
||||
preserved even when the provider resolves to openai-codex."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.delenv("LLM_MODEL", raising=False)
|
||||
monkeypatch.delenv("OPENAI_MODEL", raising=False)
|
||||
|
||||
def _runtime_resolve(**kwargs):
|
||||
return {
|
||||
"provider": "openai-codex",
|
||||
"api_mode": "codex_responses",
|
||||
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"api_key": "test-key",
|
||||
"source": "env/config",
|
||||
}
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _runtime_resolve)
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.format_runtime_provider_error", lambda exc: str(exc))
|
||||
|
||||
shell = cli.HermesCLI(model="gpt-5.1-codex-mini", compact=True, max_turns=1)
|
||||
|
||||
assert shell._model_is_default is False
|
||||
assert shell._ensure_runtime_credentials() is True
|
||||
assert shell.model == "gpt-5.1-codex-mini"
|
||||
|
||||
|
||||
def test_codex_provider_strips_provider_prefix_from_model(monkeypatch):
|
||||
"""openai/gpt-5.3-codex should become gpt-5.3-codex — the Codex
|
||||
Responses API does not accept provider-prefixed model slugs."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.delenv("LLM_MODEL", raising=False)
|
||||
monkeypatch.delenv("OPENAI_MODEL", raising=False)
|
||||
|
||||
def _runtime_resolve(**kwargs):
|
||||
return {
|
||||
"provider": "openai-codex",
|
||||
"api_mode": "codex_responses",
|
||||
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||
"api_key": "test-key",
|
||||
"source": "env/config",
|
||||
}
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _runtime_resolve)
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.format_runtime_provider_error", lambda exc: str(exc))
|
||||
|
||||
shell = cli.HermesCLI(model="openai/gpt-5.3-codex", compact=True, max_turns=1)
|
||||
|
||||
assert shell._ensure_runtime_credentials() is True
|
||||
assert shell.model == "gpt-5.3-codex"
|
||||
|
||||
|
||||
def test_cmd_model_falls_back_to_auto_on_invalid_provider(monkeypatch, capsys):
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.config.load_config",
|
||||
|
||||
@@ -259,6 +259,70 @@ class TestShellFileOpsHelpers:
|
||||
assert ops.cwd == "/"
|
||||
|
||||
|
||||
class TestSearchPathValidation:
|
||||
"""Test that search() returns an error for non-existent paths."""
|
||||
|
||||
def test_search_nonexistent_path_returns_error(self, mock_env):
|
||||
"""search() should return an error when the path doesn't exist."""
|
||||
def side_effect(command, **kwargs):
|
||||
if "test -e" in command:
|
||||
return {"output": "not_found", "returncode": 1}
|
||||
if "command -v" in command:
|
||||
return {"output": "yes", "returncode": 0}
|
||||
return {"output": "", "returncode": 0}
|
||||
mock_env.execute.side_effect = side_effect
|
||||
ops = ShellFileOperations(mock_env)
|
||||
result = ops.search("pattern", path="/nonexistent/path")
|
||||
assert result.error is not None
|
||||
assert "not found" in result.error.lower() or "Path not found" in result.error
|
||||
|
||||
def test_search_nonexistent_path_files_mode(self, mock_env):
|
||||
"""search(target='files') should also return error for bad paths."""
|
||||
def side_effect(command, **kwargs):
|
||||
if "test -e" in command:
|
||||
return {"output": "not_found", "returncode": 1}
|
||||
if "command -v" in command:
|
||||
return {"output": "yes", "returncode": 0}
|
||||
return {"output": "", "returncode": 0}
|
||||
mock_env.execute.side_effect = side_effect
|
||||
ops = ShellFileOperations(mock_env)
|
||||
result = ops.search("*.py", path="/nonexistent/path", target="files")
|
||||
assert result.error is not None
|
||||
assert "not found" in result.error.lower() or "Path not found" in result.error
|
||||
|
||||
def test_search_existing_path_proceeds(self, mock_env):
|
||||
"""search() should proceed normally when the path exists."""
|
||||
def side_effect(command, **kwargs):
|
||||
if "test -e" in command:
|
||||
return {"output": "exists", "returncode": 0}
|
||||
if "command -v" in command:
|
||||
return {"output": "yes", "returncode": 0}
|
||||
# rg returns exit 1 (no matches) with empty output
|
||||
return {"output": "", "returncode": 1}
|
||||
mock_env.execute.side_effect = side_effect
|
||||
ops = ShellFileOperations(mock_env)
|
||||
result = ops.search("pattern", path="/existing/path")
|
||||
assert result.error is None
|
||||
assert result.total_count == 0 # No matches but no error
|
||||
|
||||
def test_search_rg_error_exit_code(self, mock_env):
|
||||
"""search() should report error when rg returns exit code 2."""
|
||||
call_count = {"n": 0}
|
||||
def side_effect(command, **kwargs):
|
||||
call_count["n"] += 1
|
||||
if "test -e" in command:
|
||||
return {"output": "exists", "returncode": 0}
|
||||
if "command -v" in command:
|
||||
return {"output": "yes", "returncode": 0}
|
||||
# rg returns exit 2 (error) with empty output
|
||||
return {"output": "", "returncode": 2}
|
||||
mock_env.execute.side_effect = side_effect
|
||||
ops = ShellFileOperations(mock_env)
|
||||
result = ops.search("pattern", path="/some/path")
|
||||
assert result.error is not None
|
||||
assert "search failed" in result.error.lower() or "Search error" in result.error
|
||||
|
||||
|
||||
class TestShellFileOpsWriteDenied:
|
||||
def test_write_file_denied_path(self, file_ops):
|
||||
result = file_ops.write_file("~/.ssh/authorized_keys", "evil key")
|
||||
|
||||
@@ -819,6 +819,14 @@ class ShellFileOperations(FileOperations):
|
||||
# Expand ~ and other shell paths
|
||||
path = self._expand_path(path)
|
||||
|
||||
# Validate that the path exists before searching
|
||||
check = self._exec(f"test -e {self._escape_shell_arg(path)} && echo exists || echo not_found")
|
||||
if "not_found" in check.stdout:
|
||||
return SearchResult(
|
||||
error=f"Path not found: {path}. Verify the path exists (use 'terminal' to check).",
|
||||
total_count=0
|
||||
)
|
||||
|
||||
if target == "files":
|
||||
return self._search_files(pattern, path, limit, offset)
|
||||
else:
|
||||
@@ -919,6 +927,11 @@ class ShellFileOperations(FileOperations):
|
||||
cmd = " ".join(cmd_parts)
|
||||
result = self._exec(cmd, timeout=60)
|
||||
|
||||
# rg exit codes: 0=matches found, 1=no matches, 2=error
|
||||
if result.exit_code == 2 and not result.stdout.strip():
|
||||
error_msg = result.stderr.strip() if hasattr(result, 'stderr') and result.stderr else "Search error"
|
||||
return SearchResult(error=f"Search failed: {error_msg}", total_count=0)
|
||||
|
||||
# Parse results based on output mode
|
||||
if output_mode == "files_only":
|
||||
all_files = [f for f in result.stdout.strip().split('\n') if f]
|
||||
@@ -1013,6 +1026,11 @@ class ShellFileOperations(FileOperations):
|
||||
cmd = " ".join(cmd_parts)
|
||||
result = self._exec(cmd, timeout=60)
|
||||
|
||||
# grep exit codes: 0=matches found, 1=no matches, 2=error
|
||||
if result.exit_code == 2 and not result.stdout.strip():
|
||||
error_msg = result.stderr.strip() if hasattr(result, 'stderr') and result.stderr else "Search error"
|
||||
return SearchResult(error=f"Search failed: {error_msg}", total_count=0)
|
||||
|
||||
if output_mode == "files_only":
|
||||
all_files = [f for f in result.stdout.strip().split('\n') if f]
|
||||
total = len(all_files)
|
||||
|
||||
Reference in New Issue
Block a user