fix(update): make the git-path import check non-destructive
Phase 2 review (C2) and /simplify-code findings. C2 — the git path ran the import guard before `_clear_bytecode_cache`, wired into the syntax guard's `git reset --hard` rollback. But `cannot import name 'X'` is ALSO the documented signature of the stale-bytecode class (#6207, #60242, see _sweep_stale_bytecode_if_checkout_changed), which the very next steps — and the launch-time sweep — already self-heal. A false positive there would destroy a good update over a state that fixes itself. Remove the guard from the rollback path entirely and re-add it at the end of the git path, after bytecode sweep + dependency reinstall + lazy refresh, as a WARNING only. By then every benign source of a transient ImportError has run, and we never reset the user's checkout. W6 — the headline regression test was vacuous: it patched `hermes_main._UPDATE_CRITICAL_FILES`, but the syntax guard reads `update_cmd`'s global, so the stub files were never examined and the (True, None, None) came from "no files found" rather than "parses clean". Patch the right module; mutation-checked (the test now fails when the guard is disabled). S5 — `startswith(("tools","agent","hermes","gateway"))` also matched third-party `agents`/`agentops`/`toolsets`. Compare the first dotted segment against an exact set instead. S6 — hoist the per-line ChatConsole() instantiation.
This commit is contained in:
parent
822571fa8e
commit
bf18710a54
|
|
@ -461,11 +461,12 @@ class CLIAgentSetupMixin:
|
|||
# Keep _pending_title so it can be retried after row creation succeeds
|
||||
return True
|
||||
except Exception as e:
|
||||
ChatConsole().print(f"[bold red]Failed to initialize agent: {e}[/]")
|
||||
console = ChatConsole()
|
||||
console.print(f"[bold red]Failed to initialize agent: {e}[/]")
|
||||
from hermes_constants import partial_update_hint
|
||||
|
||||
for line in partial_update_hint(e):
|
||||
ChatConsole().print(line)
|
||||
console.print(line)
|
||||
return False
|
||||
|
||||
def _preload_resumed_session(self) -> bool:
|
||||
|
|
|
|||
|
|
@ -3819,22 +3819,9 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
syntax_ok, failing_path, syntax_error = _validate_critical_files_syntax(
|
||||
_m().PROJECT_ROOT
|
||||
)
|
||||
if syntax_ok:
|
||||
# Parsing clean isn't enough: a tree can be syntactically valid
|
||||
# and still unimportable when modules land out of sync (see
|
||||
# _validate_critical_modules_import). Catch that here too so the
|
||||
# same rollback path covers both failure modes.
|
||||
(
|
||||
syntax_ok,
|
||||
failing_path,
|
||||
syntax_error,
|
||||
) = _m()._validate_critical_modules_import(_m().PROJECT_ROOT)
|
||||
failure_label = "cannot be imported (partially-updated tree)"
|
||||
else:
|
||||
failure_label = "has a syntax error"
|
||||
if not syntax_ok:
|
||||
print()
|
||||
print(f"✗ Pulled code {failure_label} in a critical file:")
|
||||
print("✗ Pulled code has a syntax error in a critical file:")
|
||||
print(f" {failing_path}")
|
||||
if syntax_error:
|
||||
# py_compile errors can be multi-line; show the first
|
||||
|
|
@ -4015,6 +4002,24 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
# plugin.yaml-declared deps that aren't in extras (#53272, #70636).
|
||||
_m()._refresh_active_memory_provider_dependencies()
|
||||
|
||||
# Everything that can legitimately produce a transient ImportError has
|
||||
# now run (bytecode sweep, dependency reinstall, lazy refresh), so a
|
||||
# module that still won't import is real breakage. Warn only — never
|
||||
# roll back here: `cannot import name X` is also the signature of the
|
||||
# stale-bytecode class (#6207, #60242), and the launch-time sweep in
|
||||
# _sweep_stale_bytecode_if_checkout_changed() self-heals that on the
|
||||
# next run. A destructive reset would undo a good update over a state
|
||||
# that fixes itself.
|
||||
import_ok, failing_module, import_error = _validate_critical_modules_import(
|
||||
_m().PROJECT_ROOT
|
||||
)
|
||||
if not import_ok:
|
||||
print()
|
||||
print(f" ⚠ {failing_module} still fails to import after updating:")
|
||||
print(f" {import_error}")
|
||||
print(" Run `hermes update` again — if it persists, reinstall:")
|
||||
print(" https://hermes-agent.nousresearch.com")
|
||||
|
||||
node_failures = _update_node_dependencies()
|
||||
_m()._build_web_ui(_m().PROJECT_ROOT / "web")
|
||||
|
||||
|
|
|
|||
|
|
@ -1276,7 +1276,13 @@ def partial_update_hint(exc: BaseException) -> list[str]:
|
|||
if isinstance(exc, ModuleNotFoundError):
|
||||
return []
|
||||
name = getattr(exc, "name", None)
|
||||
if not name or not str(name).startswith(("tools", "agent", "hermes", "gateway")):
|
||||
# Compare the first dotted segment against an exact set: a bare
|
||||
# ``startswith`` would also match third-party ``agents``, ``agentops``,
|
||||
# ``toolsets``, etc. and blame our updater for their import errors.
|
||||
root = str(name).split(".")[0] if name else ""
|
||||
if root not in {"tools", "agent", "gateway", "plugins", "providers"} and not (
|
||||
root == "cli" or root.startswith("hermes_") or root == "hermes"
|
||||
):
|
||||
return []
|
||||
return [
|
||||
"",
|
||||
|
|
|
|||
|
|
@ -44,8 +44,13 @@ def test_syntax_guard_passes_but_import_guard_catches_skew(monkeypatch, tmp_path
|
|||
_write_skewed_tree(tmp_path, skewed=True)
|
||||
|
||||
# Both files are valid Python -- the syntax guard sees nothing wrong.
|
||||
# NOTE: patch update_cmd's global, not hermes_main's. Both modules expose
|
||||
# the name, but _validate_critical_files_syntax reads the one in its own
|
||||
# module. Patching the re-export leaves the real list in place, the stub
|
||||
# files are never looked at, and the guard returns a vacuous (True, None,
|
||||
# None) that would make this test pass no matter what the code did.
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_UPDATE_CRITICAL_FILES", ("consumer.py", "provider/thing.py")
|
||||
update_cmd, "_UPDATE_CRITICAL_FILES", ("consumer.py", "provider/thing.py")
|
||||
)
|
||||
syntax_ok, _, _ = hermes_main._validate_critical_files_syntax(tmp_path)
|
||||
assert syntax_ok, "sanity: the skewed tree must parse cleanly"
|
||||
|
|
@ -171,3 +176,20 @@ def test_import_guard_flags_missing_first_party_module(monkeypatch, tmp_path):
|
|||
assert ok is False
|
||||
assert module == "consumer"
|
||||
assert error is not None and "tools.nonexistent_module" in error
|
||||
|
||||
|
||||
@pytest.mark.parametrize("modname", ["agents", "agentops", "toolsets_x", "hermesx"])
|
||||
def test_hint_does_not_claim_partial_update_for_lookalike_third_party(modname):
|
||||
"""``startswith`` would match third-party ``agents``/``agentops`` and blame
|
||||
our updater for someone else's import error."""
|
||||
exc = ImportError("boom")
|
||||
exc.name = modname
|
||||
assert partial_update_hint(exc) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("modname", ["tools.todo_tool", "agent.context_compressor",
|
||||
"hermes_constants", "cli"])
|
||||
def test_hint_fires_for_each_first_party_root(modname):
|
||||
exc = ImportError("cannot import name 'X'")
|
||||
exc.name = modname
|
||||
assert partial_update_hint(exc), f"expected guidance for {modname}"
|
||||
|
|
|
|||
Loading…
Reference in New Issue