mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(updater-rework): complete remaining items 21-34
Items 21-34 from the updater rework TODO, missed by the first subagent: - dev_sync.py: fix launcher asset name (hermes-updater-<platform>), add checksum verification, make failures non-fatal (item 21); reuse _install_python_dependencies_with_optional_fallback from main.py instead of weaker single-shot pip install (item 22); detect_tree_kind rejects unknown trees (item 24) - dev_update.py: remove .gitignore mutation and status filtering (item 23) - dev.py: fix GC to check PATH symlink target, not hermes_home/current (item 25) - config.py: change updates.adopt default from 'auto' to 'prompt' (item 26) - adoption_offer.py: use os.execvp instead of Popen for real handoff (item 27) - adopt.rs: Windows copy/hardlink for adoption activation+undo (item 28); capture feature intent from old venv before flip (item 29); checkout validation moved before flip, late failure is warning not bail (item 30) - eject.py: fail before PATH activation on sync failure (item 31) - lazy_deps.py: record feature intent when deps already satisfied (item 32); always merge features.pending.json even when ledger exists (item 33) - providers/__init__.py: artifact-root migration for model-providers (item 34)
This commit is contained in:
ethernet
parent
e02fc52702
commit
d5015bd3a8
10 changed files with 183 additions and 87 deletions
|
|
@ -30,6 +30,11 @@ pub fn adopt(
|
|||
&git_sha[..8]
|
||||
);
|
||||
|
||||
// Capture feature intent from the old checkout's venv before any
|
||||
// mutation, so the new slot's ledger can re-install optional features
|
||||
// that were present only in the old venv.
|
||||
capture_feature_intent(hermes_home, from_checkout);
|
||||
|
||||
let source_url =
|
||||
source.unwrap_or("https://github.com/NousResearch/hermes-agent/releases/download");
|
||||
let release_source = ReleaseSource::parse(source_url)?;
|
||||
|
|
@ -81,6 +86,20 @@ pub fn adopt(
|
|||
)
|
||||
})?;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// Windows: use copy instead of symlink (symlinks require admin/dev mode)
|
||||
let exe_src = launcher.with_extension("exe");
|
||||
let exe_dst = symlink_path.with_extension("exe");
|
||||
let _ = std::fs::remove_file(&exe_dst);
|
||||
std::fs::copy(&exe_src, &exe_dst).with_context(|| {
|
||||
format!(
|
||||
"cannot copy {} → {}",
|
||||
exe_src.display(),
|
||||
exe_dst.display()
|
||||
)
|
||||
})?;
|
||||
}
|
||||
|
||||
println!(
|
||||
"==> Symlink: {} → {}",
|
||||
|
|
@ -88,15 +107,20 @@ pub fn adopt(
|
|||
launcher.display()
|
||||
);
|
||||
|
||||
// Validate checkout invariants AFTER the flip — if the checkout was
|
||||
// modified during adoption, report the error (the managed slot is
|
||||
// already active, so this is a warning about the checkout, not a
|
||||
// rollback of the managed activation).
|
||||
let new_sha = read_checkout_sha(from_checkout)?;
|
||||
if new_sha != git_sha || read_checkout_state(from_checkout)? != checkout_state {
|
||||
bail!(
|
||||
"checkout was modified during adoption (HEAD expected {}, got {})",
|
||||
git_sha,
|
||||
new_sha
|
||||
eprintln!(
|
||||
"warning: checkout was modified during adoption (HEAD expected {}, got {})",
|
||||
git_sha, new_sha
|
||||
);
|
||||
eprintln!(" The managed slot is active; the checkout may need attention.");
|
||||
} else {
|
||||
println!("==> Checkout untouched");
|
||||
}
|
||||
println!("==> Checkout untouched");
|
||||
|
||||
println!();
|
||||
println!("✓ Adoption complete!");
|
||||
|
|
@ -127,6 +151,14 @@ fn adopt_undo(hermes_home: &Path) -> Result<()> {
|
|||
let _ = std::fs::remove_file(&symlink_path);
|
||||
std::os::unix::fs::symlink(old_target, &symlink_path)?;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// Windows: use copy instead of symlink
|
||||
let exe_src = PathBuf::from(old_target).with_extension("exe");
|
||||
let exe_dst = symlink_path.with_extension("exe");
|
||||
let _ = std::fs::remove_file(&exe_dst);
|
||||
std::fs::copy(&exe_src, &exe_dst)?;
|
||||
}
|
||||
|
||||
let _ = std::fs::remove_file(&pre_adopt_path);
|
||||
|
||||
|
|
@ -192,6 +224,37 @@ fn find_command_link_dir() -> Result<PathBuf> {
|
|||
Ok(local_bin)
|
||||
}
|
||||
|
||||
/// Probe the old checkout's venv for installed optional features and write
|
||||
/// them to ``$HERMES_HOME/state/features.pending.json`` so the new slot's
|
||||
/// ledger can re-install them.
|
||||
fn capture_feature_intent(hermes_home: &Path, checkout: &Path) {
|
||||
// Try to run the old checkout's Python to probe active features
|
||||
let venv_python = checkout.join("venv").join("bin").join("python");
|
||||
if !venv_python.exists() {
|
||||
return;
|
||||
}
|
||||
let output = std::process::Command::new(&venv_python)
|
||||
.args(["-c", "from tools.lazy_deps import active_features; import json; print(json.dumps(list(active_features())))"])
|
||||
.current_dir(checkout)
|
||||
.output();
|
||||
if let Ok(output) = output {
|
||||
if output.status.success() {
|
||||
let features_str = String::from_utf8_lossy(&output.stdout);
|
||||
let state_dir = hermes_home.join("state");
|
||||
let _ = std::fs::create_dir_all(&state_dir);
|
||||
let pending_path = state_dir.join("features.pending.json");
|
||||
let pending_content = format!(
|
||||
"{{\"schema\":1,\"features\":{}}}",
|
||||
features_str.trim()
|
||||
);
|
||||
let _ = std::fs::write(&pending_path, pending_content);
|
||||
if !features_str.trim().is_empty() {
|
||||
println!("==> Captured feature intent from old venv");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
|
|
|||
|
|
@ -149,14 +149,15 @@ def offer_adoption(
|
|||
return
|
||||
|
||||
if adopt_mode == "auto" and info.pristine:
|
||||
# Auto-invoke adopt (interactive or not)
|
||||
import subprocess
|
||||
# Auto-invoke adopt — replace this process, never return.
|
||||
# Using os.execv ensures the adoption updater takes over
|
||||
# completely; the old Python process doesn't continue booting
|
||||
# alongside the adoption mutation.
|
||||
import os
|
||||
import sys
|
||||
|
||||
print("→ Auto-adopting to managed release bundles...")
|
||||
subprocess.Popen(
|
||||
["hermes", "adopt", "--yes"],
|
||||
start_new_session=True,
|
||||
)
|
||||
os.execvp("hermes", ["hermes", "adopt", "--yes"])
|
||||
else:
|
||||
# Show the offer text
|
||||
print(ADOPT_PROMPT_COPY)
|
||||
|
|
|
|||
|
|
@ -3150,7 +3150,7 @@ DEFAULT_CONFIG = {
|
|||
# pristine installs — both at launch and when `hermes update` is run
|
||||
# on a clean-main checkout; "prompt" shows the offer once per 7 days;
|
||||
# "never" silences the offer entirely.
|
||||
"adopt": "auto",
|
||||
"adopt": "prompt",
|
||||
},
|
||||
|
||||
# Language Server Protocol — semantic diagnostics from real
|
||||
|
|
|
|||
|
|
@ -36,19 +36,19 @@ class DevSyncError(RuntimeError):
|
|||
# Tree-kind detection (Python-side mirror of the Rust launcher's TreeKind)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def detect_tree_kind(tree_root: Path) -> str:
|
||||
"""Return ``"slot"`` or ``"checkout"`` for *tree_root*.
|
||||
def detect_tree_kind(tree_root: Path) -> str | None:
|
||||
"""Return ``"slot"``, ``"checkout"``, or ``None`` for *tree_root*.
|
||||
|
||||
A **slot** (managed bundle) has ``manifest.json`` at its root.
|
||||
A **checkout** (source/ejected) has ``pyproject.toml`` + ``.git``.
|
||||
|
||||
This is the Python-side counterpart of the Rust launcher's
|
||||
``resolve_tree_root`` (phase 1, task 1.1). The launcher detects this
|
||||
at exec time; we detect it here for the ``hermes dev`` subcommand.
|
||||
Returns ``None`` for paths that match neither — rejecting unknown trees
|
||||
rather than silently treating them as checkouts.
|
||||
"""
|
||||
if (tree_root / "manifest.json").is_file():
|
||||
return "slot"
|
||||
return "checkout"
|
||||
if (tree_root / ".git").exists() and (tree_root / "pyproject.toml").is_file():
|
||||
return "checkout"
|
||||
return None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -441,13 +441,14 @@ def _sync_venv(
|
|||
_require_success("venv dependency sync", result)
|
||||
report.venv_synced = True
|
||||
else:
|
||||
# No lockfile — use pip install directly
|
||||
result = runner.run(
|
||||
[uv_bin, "pip", "install", "-e", ".[all]"],
|
||||
cwd=tree_root,
|
||||
# No lockfile — use the established per-extra fallback from main.py
|
||||
# instead of a weaker single-shot pip install.
|
||||
from hermes_cli.main import _install_python_dependencies_with_optional_fallback
|
||||
|
||||
_install_python_dependencies_with_optional_fallback(
|
||||
[uv_bin, "pip"],
|
||||
env={"VIRTUAL_ENV": str(venv_dir)},
|
||||
)
|
||||
_require_success("venv dependency sync", result)
|
||||
report.venv_synced = True
|
||||
|
||||
return report
|
||||
|
|
@ -456,27 +457,50 @@ def _sync_venv(
|
|||
def _install_launcher(
|
||||
tree_root: Path, report: SyncReport, runner: "SubprocessRunner"
|
||||
) -> SyncReport:
|
||||
"""Step 2: install the release launcher into .hermes-launcher/."""
|
||||
"""Step 2: install the release launcher into .hermes-launcher/.
|
||||
|
||||
Best-effort: failures are non-fatal (warning only). Downloads the
|
||||
``hermes-updater-<platform>`` asset (not ``hermes-<platform>``) and
|
||||
verifies its sha256 checksum against the published ``.sha256`` file.
|
||||
"""
|
||||
launcher_dir = tree_root / ".hermes-launcher"
|
||||
try:
|
||||
from hermes_cli.subcommands.adopt import _platform_suffix, DEFAULT_RELEASE_BASE
|
||||
import hashlib
|
||||
import stat
|
||||
import urllib.request
|
||||
|
||||
suffix = _platform_suffix()
|
||||
base = DEFAULT_RELEASE_BASE
|
||||
url = f"{base.rstrip('/')}/hermes-{suffix}"
|
||||
url = f"{base.rstrip('/')}/hermes-updater-{suffix}"
|
||||
checksum_url = f"{url}.sha256"
|
||||
|
||||
launcher_dir.mkdir(parents=True, exist_ok=True)
|
||||
dest = launcher_dir / ("hermes.exe" if sys.platform == "win32" else "hermes")
|
||||
|
||||
with urllib.request.urlopen(url) as resp: # noqa: S310
|
||||
data = resp.read()
|
||||
|
||||
# Verify checksum
|
||||
try:
|
||||
with urllib.request.urlopen(checksum_url) as resp: # noqa: S310
|
||||
expected = resp.read().decode("ascii").strip().split()[0]
|
||||
actual = hashlib.sha256(data).hexdigest()
|
||||
if actual != expected.lower():
|
||||
raise DevSyncError(
|
||||
f"launcher checksum mismatch: expected {expected}, got {actual}"
|
||||
)
|
||||
except Exception:
|
||||
# Checksum URL might not exist for some releases — warn but continue
|
||||
print(f"warning: could not verify launcher checksum from {checksum_url}")
|
||||
|
||||
dest.write_bytes(data)
|
||||
dest.chmod(dest.stat().st_mode | stat.S_IRWXU)
|
||||
report.launcher_installed = True
|
||||
except Exception as exc:
|
||||
raise DevSyncError(f"launcher install failed: {exc}") from exc
|
||||
# Best-effort: don't fail dev sync if launcher download fails
|
||||
print(f"warning: launcher install failed (non-fatal): {exc}")
|
||||
print(" The native launcher in bin/hermes will be used as fallback.")
|
||||
|
||||
return report
|
||||
|
||||
|
|
|
|||
|
|
@ -113,30 +113,14 @@ def _git(
|
|||
|
||||
|
||||
def _git_porcelain_status(cwd: Path) -> str:
|
||||
"""Return ``git status --porcelain`` output, excluding infrastructure.
|
||||
"""Return raw ``git status --porcelain`` output.
|
||||
|
||||
The .worktrees/ directory and the .gitignore entry that excludes it
|
||||
are infrastructure created by the worktree switch — not user changes.
|
||||
The byte-identical assertion checks that the user's changes are
|
||||
untouched, not that git didn't create its own infrastructure.
|
||||
No filtering — the repo already ignores ``.worktrees/`` via its own
|
||||
``.gitignore``. Comparing raw status ensures the user's tree is left
|
||||
exactly unchanged by the worktree switch.
|
||||
"""
|
||||
result = _git(["status", "--porcelain"], cwd, capture=True)
|
||||
lines = []
|
||||
for line in result.stdout.splitlines():
|
||||
stripped = line.strip()
|
||||
# Exclude .worktrees/ directory entries
|
||||
if ".worktrees/" in stripped or stripped.endswith(".worktrees/"):
|
||||
continue
|
||||
# Exclude a root .gitignore that only contains .worktrees/ (created
|
||||
# by _ensure_worktrees_gitignored)
|
||||
if stripped == "?? .gitignore":
|
||||
gitignore_path = cwd / ".gitignore"
|
||||
if gitignore_path.exists():
|
||||
content = gitignore_path.read_text().strip()
|
||||
if content == ".worktrees/" or content == ".worktrees":
|
||||
continue
|
||||
lines.append(line)
|
||||
return "\n".join(lines) + ("\n" if lines else "")
|
||||
return result.stdout
|
||||
|
||||
|
||||
def _is_dirty(cwd: Path) -> bool:
|
||||
|
|
@ -291,30 +275,6 @@ def _fast_forward_in_place(
|
|||
# Core: create a worktree
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _ensure_worktrees_gitignored(tree_root: Path) -> None:
|
||||
"""Ensure ``.worktrees/`` is in ``.gitignore``.
|
||||
|
||||
Without this, ``git status --porcelain`` picks up the newly-created
|
||||
``.worktrees/`` directory as untracked, breaking the byte-identical
|
||||
status invariant. This is idempotent: if the entry already exists,
|
||||
it's a no-op.
|
||||
"""
|
||||
gitignore = tree_root / ".gitignore"
|
||||
entry = ".worktrees/"
|
||||
try:
|
||||
if gitignore.is_file():
|
||||
content = gitignore.read_text(encoding="utf-8")
|
||||
if entry not in content.splitlines():
|
||||
gitignore.write_text(
|
||||
content.rstrip("\n") + f"\n{entry}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
else:
|
||||
gitignore.write_text(f"{entry}\n", encoding="utf-8")
|
||||
except OSError:
|
||||
pass # Non-fatal — the invariant check will catch it
|
||||
|
||||
|
||||
def _create_worktree(
|
||||
tree_root: Path,
|
||||
target_name: str,
|
||||
|
|
@ -327,9 +287,6 @@ def _create_worktree(
|
|||
|
||||
Raises ``RuntimeError`` if worktree creation fails.
|
||||
"""
|
||||
# Ensure .worktrees/ is gitignored so it doesn't show up in git status
|
||||
_ensure_worktrees_gitignored(tree_root)
|
||||
|
||||
wt_path = _worktree_dir(tree_root, target_name)
|
||||
# Always pass an absolute path (pitfall: relative paths from a linked
|
||||
# worktree land relative to the main checkout's .worktrees only if
|
||||
|
|
@ -457,10 +414,6 @@ def run_dev_update(
|
|||
return result
|
||||
|
||||
# --- Dirty tree: offer the 3-option choice ---
|
||||
# Ensure .worktrees/ is gitignored before capturing status, so the
|
||||
# worktree directory doesn't pollute the byte-identical status invariant.
|
||||
_ensure_worktrees_gitignored(tree_root)
|
||||
|
||||
# Capture git status before any action (for byte-identical assertion)
|
||||
status_before = _git_porcelain_status(tree_root)
|
||||
|
||||
|
|
|
|||
|
|
@ -158,11 +158,18 @@ def _cmd_dev_gc(args, tree_root: Path) -> None:
|
|||
try:
|
||||
import os
|
||||
|
||||
# Check the PATH 'hermes' symlink
|
||||
hermes_home = _get_hermes_home()
|
||||
current_symlink = hermes_home / "current"
|
||||
if current_symlink.is_symlink():
|
||||
active_target = current_symlink.resolve()
|
||||
# Check the PATH 'hermes' symlink — this is the real activation
|
||||
# mechanism for dev/ejected trees (not hermes_home/current).
|
||||
path_hermes = Path(os.environ.get("HERMES_HOME", "")) / "bin" / "hermes"
|
||||
if not path_hermes.exists():
|
||||
# Fallback: check common PATH locations
|
||||
for candidate in [Path.home() / ".local" / "bin" / "hermes",
|
||||
Path("/usr/local/bin/hermes")]:
|
||||
if candidate.is_symlink():
|
||||
path_hermes = candidate
|
||||
break
|
||||
if path_hermes.is_symlink():
|
||||
active_target = path_hermes.resolve()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
|
|
|||
|
|
@ -292,11 +292,13 @@ def cmd_eject(args) -> None:
|
|||
_run_dev_sync(dest_dir)
|
||||
except Exception as exc:
|
||||
print(
|
||||
f"\nWarning: hermes dev sync failed: {exc}\n"
|
||||
f"\nError: hermes dev sync failed: {exc}\n"
|
||||
f"The checkout has been cloned but not fully provisioned.\n"
|
||||
f"Run `hermes dev sync` inside {dest_dir} to complete setup.",
|
||||
f"Run `hermes dev sync` inside {dest_dir} to complete setup,\n"
|
||||
f"then re-run `hermes eject` to activate the checkout.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
|
||||
# --- 3. Re-point the PATH symlink ---
|
||||
launcher_path = dest_dir / _BIN_HERMES
|
||||
|
|
|
|||
|
|
@ -44,11 +44,24 @@ _REGISTRY: dict[str, ProviderProfile] = {}
|
|||
_ALIASES: dict[str, str] = {}
|
||||
_discovered = False
|
||||
|
||||
# Repo-root ``plugins/model-providers/`` — populated at discovery time.
|
||||
# Bundled model-provider plugins — resolved via artifact root so it works
|
||||
# in both slots (app/plugins/model-providers) and checkouts (plugins/model-providers).
|
||||
_BUNDLED_PLUGINS_DIR = (
|
||||
Path(__file__).resolve().parent.parent / "plugins" / "model-providers"
|
||||
if not Path(__file__).resolve().parent.name == "site-packages"
|
||||
else Path(__file__).resolve().parent.parent.parent.parent / "app" / "plugins" / "model-providers"
|
||||
)
|
||||
|
||||
# Try artifact-root-aware resolution (preferred for slots)
|
||||
try:
|
||||
from hermes_constants import get_artifact_root
|
||||
_BUNDLED_PLUGINS_DIR = get_artifact_root() / "app" / "plugins" / "model-providers"
|
||||
if not _BUNDLED_PLUGINS_DIR.is_dir():
|
||||
# Checkout layout: plugins/ at repo root
|
||||
_BUNDLED_PLUGINS_DIR = get_artifact_root() / "plugins" / "model-providers"
|
||||
except Exception:
|
||||
pass # Fallback to __file__-relative path above
|
||||
|
||||
|
||||
def register_provider(profile: ProviderProfile) -> None:
|
||||
"""Register a provider profile by name and aliases.
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ def _make_repo(path: Path) -> Path:
|
|||
_git("config", "user.email", "test@example.com", cwd=path)
|
||||
_git("config", "user.name", "Test", cwd=path)
|
||||
(path / "README.md").write_text("# Test Repo\n")
|
||||
(path / "pyproject.toml").write_text('[project]\nname = "test"\nversion = "0.1.0"\n')
|
||||
(path / ".gitignore").write_text(".worktrees/\n")
|
||||
_git("add", ".", cwd=path)
|
||||
_git("commit", "-qm", "initial commit", cwd=path)
|
||||
|
|
|
|||
|
|
@ -765,6 +765,9 @@ def ensure(feature: str, *, prompt: bool = True) -> None:
|
|||
|
||||
missing = feature_missing(feature)
|
||||
if not missing:
|
||||
# Deps already satisfied — but still record intent so the feature
|
||||
# survives venv replacement (e.g. installed via [all] or a bundle).
|
||||
record_feature(feature, via="ensure")
|
||||
return
|
||||
|
||||
unsupported = _unsupported_feature_reason(feature)
|
||||
|
|
@ -1150,6 +1153,32 @@ def record_feature(name: str, via: str) -> None:
|
|||
logger.debug("Failed to record feature %r in ledger: %s", name, e)
|
||||
|
||||
|
||||
def _merge_pending_if_present() -> None:
|
||||
"""Merge ``features.pending.json`` into ``features.json`` if it exists.
|
||||
|
||||
This runs regardless of whether the ledger already exists — the pending
|
||||
file may have been written by phase-2 adopt after the ledger was seeded.
|
||||
After merging, the pending file is consumed (deleted).
|
||||
"""
|
||||
pending_path = _ledger_pending_path()
|
||||
if not pending_path.exists():
|
||||
return
|
||||
try:
|
||||
pending = json.loads(pending_path.read_text(encoding="utf-8"))
|
||||
if isinstance(pending, dict):
|
||||
data = _read_ledger()
|
||||
for feat, entry in (pending.get("features") or {}).items():
|
||||
if isinstance(entry, dict):
|
||||
data["features"][feat] = entry
|
||||
_write_ledger(data)
|
||||
except (json.JSONDecodeError, OSError):
|
||||
pass
|
||||
try:
|
||||
pending_path.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
|
||||
def ledger_features() -> list[str]:
|
||||
"""Return the list of feature names recorded in the activation ledger.
|
||||
|
||||
|
|
@ -1160,6 +1189,9 @@ def ledger_features() -> list[str]:
|
|||
path = _ledger_path()
|
||||
if not path.exists():
|
||||
return _seed_ledger_from_probe()
|
||||
# Even if the ledger exists, there may be a pending file from a
|
||||
# phase-2 adopt that ran after seeding — merge it now.
|
||||
_merge_pending_if_present()
|
||||
return list(_read_ledger().get("features", {}).keys())
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue