From 475f71f0f96e34c21e350ba4d2f46eb9333d5e21 Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 09:56:29 -0700 Subject: [PATCH 1/8] Add mind branch setup, parent lineage tracking, and update command When creating a new mind, after cloning the repo: 1. Record the parent branch and commit hash 2. Checkout a new branch named minds/ 3. Write a .parent file (git config format) tracking the parent URL, branch, and commit hash 4. Commit the .parent file Add a "mind update " CLI command that: A) Stops the mind (mng stop) B) Fetches and merges the latest code from the parent repository C) Updates all vendored git subtrees (git subtree pull) D) Starts the mind back up (mng start) Also makes run_git and ensure_git_identity public in vendor_mng.py so they can be reused by parent_tracking.py. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/minds/imbue/minds/cli/update.py | 151 ++++++++++ apps/minds/imbue/minds/cli/update_test.py | 39 +++ .../minds/forwarding_server/agent_creator.py | 4 + .../forwarding_server/parent_tracking.py | 257 ++++++++++++++++++ .../forwarding_server/parent_tracking_test.py | 146 ++++++++++ .../minds/forwarding_server/vendor_mng.py | 73 ++++- .../forwarding_server/vendor_mng_test.py | 52 ++++ apps/minds/imbue/minds/main.py | 2 + 8 files changed, 715 insertions(+), 9 deletions(-) create mode 100644 apps/minds/imbue/minds/cli/update.py create mode 100644 apps/minds/imbue/minds/cli/update_test.py create mode 100644 apps/minds/imbue/minds/forwarding_server/parent_tracking.py create mode 100644 apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py diff --git a/apps/minds/imbue/minds/cli/update.py b/apps/minds/imbue/minds/cli/update.py new file mode 100644 index 000000000..2284e34f2 --- /dev/null +++ b/apps/minds/imbue/minds/cli/update.py @@ -0,0 +1,151 @@ +"""CLI command for updating an existing mind with the latest parent code. + +The ``mind update `` command: + +1. Stops the mind (via ``mng stop``) +2. Fetches and merges the latest code from the parent repository +3. Updates all vendored git subtrees +4. Starts the mind back up (via ``mng start``) +""" + +import json +from pathlib import Path +from typing import Final + +import click +from loguru import logger + +from imbue.concurrency_group.concurrency_group import ConcurrencyGroup +from imbue.minds.config.data_types import MNG_BINARY +from imbue.minds.errors import MindError +from imbue.minds.errors import MngCommandError +from imbue.minds.forwarding_server.agent_creator import load_creation_settings +from imbue.minds.forwarding_server.parent_tracking import fetch_and_merge_parent +from imbue.minds.forwarding_server.parent_tracking import read_parent_info +from imbue.minds.forwarding_server.vendor_mng import default_vendor_configs +from imbue.minds.forwarding_server.vendor_mng import find_mng_repo_root +from imbue.minds.forwarding_server.vendor_mng import update_vendor_repos +from imbue.mng.primitives import AgentId + +_MNG_COMMAND_TIMEOUT_SECONDS: Final[float] = 120.0 + + +def find_mind_agent(agent_name: str) -> dict[str, object]: + """Find a mind agent by name using ``mng list``. + + Searches for agents with the label ``mind=``. + Returns the full agent record dict from the JSON output. + + Raises MindError if the agent cannot be found. + """ + cg = ConcurrencyGroup(name="mng-list") + with cg: + result = cg.run_process_to_completion( + command=[MNG_BINARY, "list", "--label", "mind={}".format(agent_name), "--format=json"], + is_checked_after=False, + ) + if result.returncode != 0: + raise MindError( + "Failed to list agents: {}".format( + result.stderr.strip() if result.stderr.strip() else result.stdout.strip() + ) + ) + + agents = _parse_agents_from_output(result.stdout) + if not agents: + raise MindError("No mind found with name '{}'".format(agent_name)) + + return agents[0] + + +def _parse_agents_from_output(stdout: str) -> list[dict[str, object]]: + """Parse agent records from ``mng list --format json`` output. + + Handles the case where stdout may contain non-JSON lines + (e.g. SSH error tracebacks) mixed with the JSON output. + """ + for line in stdout.splitlines(): + stripped = line.strip() + if stripped.startswith("{"): + try: + data = json.loads(stripped) + return list(data.get("agents", [])) + except json.JSONDecodeError: + continue + return [] + + +def run_mng_stop(agent_id: AgentId) -> None: + """Stop a mind agent via ``mng stop``. + + Raises MngCommandError if the command fails. + """ + logger.info("Stopping agent {}...", agent_id) + cg = ConcurrencyGroup(name="mng-stop") + with cg: + result = cg.run_process_to_completion( + command=[MNG_BINARY, "stop", str(agent_id)], + is_checked_after=False, + ) + if result.returncode != 0: + raise MngCommandError( + "mng stop failed (exit code {}):\n{}".format( + result.returncode, + result.stderr.strip() if result.stderr.strip() else result.stdout.strip(), + ) + ) + + +def run_mng_start(agent_id: AgentId) -> None: + """Start a mind agent via ``mng start``. + + Raises MngCommandError if the command fails. + """ + logger.info("Starting agent {}...", agent_id) + cg = ConcurrencyGroup(name="mng-start") + with cg: + result = cg.run_process_to_completion( + command=[MNG_BINARY, "start", str(agent_id)], + is_checked_after=False, + ) + if result.returncode != 0: + raise MngCommandError( + "mng start failed (exit code {}):\n{}".format( + result.returncode, + result.stderr.strip() if result.stderr.strip() else result.stdout.strip(), + ) + ) + + +@click.command() +@click.argument("agent_name") +def update(agent_name: str) -> None: + """Update a mind with the latest code from its parent repository. + + Stops the mind, merges the latest parent code, updates vendored + subtrees, and starts the mind back up. + """ + logger.info("Looking up mind '{}'...", agent_name) + agent_record = find_mind_agent(agent_name) + agent_id = AgentId(str(agent_record["id"])) + work_dir = Path(str(agent_record["work_dir"])) + + logger.info("Found mind '{}' (agent_id={}, work_dir={})", agent_name, agent_id, work_dir) + + run_mng_stop(agent_id) + + logger.info("Merging latest code from parent repository...") + parent_info = read_parent_info(work_dir) + new_hash = fetch_and_merge_parent(work_dir, parent_info) + logger.info("Merged parent changes (new hash: {})", new_hash[:12]) + + logger.info("Updating vendored subtrees...") + settings = load_creation_settings(work_dir) + mng_repo_root = find_mng_repo_root() + vendor_configs = settings.vendor if settings.vendor else default_vendor_configs(mng_repo_root) + update_vendor_repos(work_dir, vendor_configs) + logger.info("Vendored subtrees updated ({} configured)", len(vendor_configs)) + + run_mng_start(agent_id) + + logger.info("Mind '{}' updated successfully.", agent_name) diff --git a/apps/minds/imbue/minds/cli/update_test.py b/apps/minds/imbue/minds/cli/update_test.py new file mode 100644 index 000000000..780388c02 --- /dev/null +++ b/apps/minds/imbue/minds/cli/update_test.py @@ -0,0 +1,39 @@ +import json + +from imbue.minds.cli.update import _parse_agents_from_output + + +def test_parse_agents_from_output_extracts_records() -> None: + """Verify _parse_agents_from_output extracts agent records from JSON.""" + json_str = json.dumps( + { + "agents": [ + {"id": "agent-abc123", "name": "selene", "work_dir": "/tmp/minds/selene"}, + ] + } + ) + agents = _parse_agents_from_output(json_str) + assert len(agents) == 1 + assert agents[0]["id"] == "agent-abc123" + assert agents[0]["name"] == "selene" + + +def test_parse_agents_from_output_handles_empty() -> None: + """Verify _parse_agents_from_output returns empty list for no agents.""" + json_str = json.dumps({"agents": []}) + agents = _parse_agents_from_output(json_str) + assert agents == [] + + +def test_parse_agents_from_output_handles_non_json() -> None: + """Verify _parse_agents_from_output handles non-JSON output gracefully.""" + agents = _parse_agents_from_output("not json at all") + assert agents == [] + + +def test_parse_agents_from_output_handles_mixed_output() -> None: + """Verify _parse_agents_from_output handles SSH errors mixed with JSON.""" + output = "WARNING: some SSH error\n" + json.dumps({"agents": [{"id": "agent-xyz", "name": "test"}]}) + agents = _parse_agents_from_output(output) + assert len(agents) == 1 + assert agents[0]["id"] == "agent-xyz" diff --git a/apps/minds/imbue/minds/forwarding_server/agent_creator.py b/apps/minds/imbue/minds/forwarding_server/agent_creator.py index c4f66eab0..00cb8770e 100644 --- a/apps/minds/imbue/minds/forwarding_server/agent_creator.py +++ b/apps/minds/imbue/minds/forwarding_server/agent_creator.py @@ -32,6 +32,7 @@ from imbue.minds.errors import GitCloneError from imbue.minds.errors import MngCommandError from imbue.minds.errors import VendorError +from imbue.minds.forwarding_server.parent_tracking import setup_mind_branch_and_parent from imbue.minds.forwarding_server.vendor_mng import default_vendor_configs from imbue.minds.forwarding_server.vendor_mng import find_mng_repo_root from imbue.minds.forwarding_server.vendor_mng import vendor_repos @@ -274,6 +275,9 @@ def _create_agent_background( log_queue.put("[minds] Cloning {}...".format(git_url)) clone_git_repo(GitUrl(git_url), mind_dir, on_output=emit_log) + log_queue.put("[minds] Setting up branch and parent tracking...") + setup_mind_branch_and_parent(mind_dir, agent_name, git_url, on_output=emit_log) + settings = load_creation_settings(mind_dir) mng_repo_root = find_mng_repo_root() diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking.py new file mode 100644 index 000000000..c70cab3af --- /dev/null +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking.py @@ -0,0 +1,257 @@ +"""Parent lineage tracking for mind repositories. + +Tracks the original repository, branch, and commit hash that a mind was +created from by writing a ``.parent`` file (git config format) in the +mind's root directory. + +The ``.parent`` file records three values under the ``[parent]`` section:: + + [parent] + url = https://github.com/org/repo.git + branch = main + hash = abc123... + +This information is used by the ``mind update`` command to fetch and merge +the latest changes from the parent repository. +""" + +from collections.abc import Callable +from pathlib import Path +from typing import Final + +from loguru import logger +from pydantic import Field + +from imbue.imbue_common.frozen_model import FrozenModel +from imbue.minds.forwarding_server.vendor_mng import ensure_git_identity +from imbue.minds.forwarding_server.vendor_mng import run_git + +PARENT_FILE_NAME: Final[str] = ".parent" + +MIND_BRANCH_PREFIX: Final[str] = "minds/" + + +class ParentInfo(FrozenModel): + """Lineage information for a mind's parent repository. + + Stores the URL, branch, and commit hash of the repository that was + cloned to create the mind. + """ + + url: str = Field(description="Git URL of the parent repository") + branch: str = Field(description="Branch name in the parent repository") + hash: str = Field(description="Commit hash at the time of creation or last update") + + +def get_current_branch(repo_dir: Path) -> str: + """Return the current branch name of a git repository. + + Returns ``HEAD`` if the repository is in detached HEAD state. + """ + return run_git( + ["rev-parse", "--abbrev-ref", "HEAD"], + cwd=repo_dir, + error_message="Failed to get current branch of {}".format(repo_dir), + ).strip() + + +def get_current_commit_hash(repo_dir: Path) -> str: + """Return the full commit hash of HEAD in a git repository.""" + return run_git( + ["rev-parse", "HEAD"], + cwd=repo_dir, + error_message="Failed to get current commit hash of {}".format(repo_dir), + ).strip() + + +def checkout_mind_branch( + repo_dir: Path, + mind_name: str, + on_output: Callable[[str, bool], None] | None = None, +) -> None: + """Create and switch to a new branch named ``minds/``. + + Raises VendorError if the branch already exists or the checkout fails. + """ + branch_name = "{}{}".format(MIND_BRANCH_PREFIX, mind_name) + logger.debug("Checking out new branch: {}", branch_name) + run_git( + ["checkout", "-b", branch_name], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to create branch {}".format(branch_name), + ) + + +def write_parent_info( + repo_dir: Path, + parent_info: ParentInfo, + on_output: Callable[[str, bool], None] | None = None, +) -> None: + """Write parent lineage information to the ``.parent`` file. + + Uses ``git config --file`` to write a git-config-formatted file. + Overwrites any existing ``.parent`` file. + """ + parent_file = str(repo_dir / PARENT_FILE_NAME) + logger.debug( + "Writing parent info to {}: url={}, branch={}, hash={}", + parent_file, + parent_info.url, + parent_info.branch, + parent_info.hash, + ) + + run_git( + ["config", "--file", parent_file, "parent.url", parent_info.url], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to write parent.url to {}".format(parent_file), + ) + run_git( + ["config", "--file", parent_file, "parent.branch", parent_info.branch], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to write parent.branch to {}".format(parent_file), + ) + run_git( + ["config", "--file", parent_file, "parent.hash", parent_info.hash], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to write parent.hash to {}".format(parent_file), + ) + + +def read_parent_info(repo_dir: Path) -> ParentInfo: + """Read parent lineage information from the ``.parent`` file. + + Raises VendorError if the file does not exist or cannot be read. + """ + parent_file = str(repo_dir / PARENT_FILE_NAME) + + url = run_git( + ["config", "--file", parent_file, "parent.url"], + cwd=repo_dir, + error_message="Failed to read parent.url from {}".format(parent_file), + ).strip() + branch = run_git( + ["config", "--file", parent_file, "parent.branch"], + cwd=repo_dir, + error_message="Failed to read parent.branch from {}".format(parent_file), + ).strip() + hash_value = run_git( + ["config", "--file", parent_file, "parent.hash"], + cwd=repo_dir, + error_message="Failed to read parent.hash from {}".format(parent_file), + ).strip() + + return ParentInfo(url=url, branch=branch, hash=hash_value) + + +def commit_parent_file( + repo_dir: Path, + on_output: Callable[[str, bool], None] | None = None, +) -> None: + """Stage and commit the ``.parent`` file if it has changes. + + Creates a commit with a descriptive message. Ensures git identity + is configured before committing. Skips the commit if the file has + no staged changes (e.g. when the content is unchanged). + """ + ensure_git_identity(repo_dir) + run_git( + ["add", PARENT_FILE_NAME], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to stage {}".format(PARENT_FILE_NAME), + ) + + # Check if there are staged changes before committing + diff_output = run_git( + ["diff", "--cached", "--name-only"], + cwd=repo_dir, + error_message="Failed to check staged changes", + ).strip() + + if not diff_output: + logger.debug("No changes to .parent file, skipping commit") + return + + run_git( + ["commit", "-m", "Record parent repository lineage"], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to commit {}".format(PARENT_FILE_NAME), + ) + + +def setup_mind_branch_and_parent( + repo_dir: Path, + mind_name: str, + git_url: str, + on_output: Callable[[str, bool], None] | None = None, +) -> None: + """Set up a mind's branch and parent tracking after cloning. + + Performs the following steps: + 1. Records the current branch and commit hash (the parent info) + 2. Creates and switches to a ``minds/`` branch + 3. Writes the ``.parent`` file with the parent lineage + 4. Commits the ``.parent`` file + """ + parent_branch = get_current_branch(repo_dir) + parent_hash = get_current_commit_hash(repo_dir) + + logger.debug("Parent branch: {}, hash: {}", parent_branch, parent_hash) + + checkout_mind_branch(repo_dir, mind_name, on_output) + + parent_info = ParentInfo(url=git_url, branch=parent_branch, hash=parent_hash) + write_parent_info(repo_dir, parent_info, on_output) + commit_parent_file(repo_dir, on_output) + + +def fetch_and_merge_parent( + repo_dir: Path, + parent_info: ParentInfo, + on_output: Callable[[str, bool], None] | None = None, +) -> str: + """Fetch the latest code from the parent repository and merge it. + + Fetches the parent branch and merges FETCH_HEAD into the current branch. + After merging, updates the ``.parent`` file with the new commit hash + and commits the change. + + Returns the new parent commit hash (the fetched HEAD). + + Raises VendorError if the fetch or merge fails (e.g. due to conflicts). + """ + ensure_git_identity(repo_dir) + + logger.debug("Fetching from {} branch {}", parent_info.url, parent_info.branch) + run_git( + ["fetch", parent_info.url, parent_info.branch], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to fetch from {} branch {}".format(parent_info.url, parent_info.branch), + ) + + new_hash = run_git( + ["rev-parse", "FETCH_HEAD"], + cwd=repo_dir, + error_message="Failed to resolve FETCH_HEAD", + ).strip() + + logger.debug("Merging FETCH_HEAD ({})", new_hash) + run_git( + ["merge", "FETCH_HEAD", "--no-edit"], + cwd=repo_dir, + on_output=on_output, + error_message="Failed to merge parent changes (there may be conflicts to resolve manually)", + ) + + updated_info = ParentInfo(url=parent_info.url, branch=parent_info.branch, hash=new_hash) + write_parent_info(repo_dir, updated_info, on_output) + commit_parent_file(repo_dir, on_output) + + return new_hash diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py new file mode 100644 index 000000000..e4dcddefb --- /dev/null +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py @@ -0,0 +1,146 @@ +from pathlib import Path + +import pytest + +from imbue.minds.errors import VendorError +from imbue.minds.forwarding_server.parent_tracking import MIND_BRANCH_PREFIX +from imbue.minds.forwarding_server.parent_tracking import PARENT_FILE_NAME +from imbue.minds.forwarding_server.parent_tracking import ParentInfo +from imbue.minds.forwarding_server.parent_tracking import checkout_mind_branch +from imbue.minds.forwarding_server.parent_tracking import commit_parent_file +from imbue.minds.forwarding_server.parent_tracking import fetch_and_merge_parent +from imbue.minds.forwarding_server.parent_tracking import get_current_branch +from imbue.minds.forwarding_server.parent_tracking import get_current_commit_hash +from imbue.minds.forwarding_server.parent_tracking import read_parent_info +from imbue.minds.forwarding_server.parent_tracking import setup_mind_branch_and_parent +from imbue.minds.forwarding_server.parent_tracking import write_parent_info +from imbue.minds.forwarding_server.vendor_mng import run_git +from imbue.minds.testing import add_and_commit_git_repo +from imbue.minds.testing import init_and_commit_git_repo + + +def _make_repo(tmp_path: Path, name: str = "repo") -> Path: + """Create a minimal git repo with a committed file.""" + repo = tmp_path / name + repo.mkdir() + (repo / "hello.txt").write_text("hello") + init_and_commit_git_repo(repo, tmp_path) + return repo + + +def test_get_current_branch_returns_branch_name(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + branch = get_current_branch(repo) + # Default branch is typically "master" or "main" depending on git config + assert branch in ("master", "main") + + +def test_get_current_commit_hash_returns_full_hash(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + commit_hash = get_current_commit_hash(repo) + assert len(commit_hash) == 40 + assert all(c in "0123456789abcdef" for c in commit_hash) + + +def test_checkout_mind_branch_creates_new_branch(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + checkout_mind_branch(repo, "selene") + branch = get_current_branch(repo) + assert branch == "{}selene".format(MIND_BRANCH_PREFIX) + + +def test_write_and_read_parent_info_roundtrips(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + parent = ParentInfo( + url="https://github.com/org/repo.git", + branch="main", + hash="abc123def456" * 3 + "abcd", + ) + write_parent_info(repo, parent) + result = read_parent_info(repo) + assert result == parent + + +def test_read_parent_info_raises_when_file_missing(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + with pytest.raises(VendorError, match="Failed to read parent.url"): + read_parent_info(repo) + + +def test_commit_parent_file_creates_commit(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + parent = ParentInfo(url="https://example.com/repo.git", branch="main", hash="a" * 40) + write_parent_info(repo, parent) + commit_parent_file(repo) + + log_output = run_git( + ["log", "--oneline", "-1"], + cwd=repo, + error_message="Failed to read git log", + ) + assert "parent" in log_output.lower() + + +def test_setup_mind_branch_and_parent_full_flow(tmp_path: Path) -> None: + """Verify the full setup flow: branch creation, parent tracking, and commit.""" + source = _make_repo(tmp_path, "source") + clone_dir = tmp_path / "clone" + + run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") + + setup_mind_branch_and_parent(clone_dir, "selene", str(source)) + + # Verify branch + branch = get_current_branch(clone_dir) + assert branch == "minds/selene" + + # Verify parent info + parent = read_parent_info(clone_dir) + assert parent.url == str(source) + assert parent.branch in ("master", "main") + assert len(parent.hash) == 40 + + # Verify .parent file is committed + assert (clone_dir / PARENT_FILE_NAME).exists() + + +def test_fetch_and_merge_parent_merges_changes(tmp_path: Path) -> None: + """Verify that fetch_and_merge_parent pulls new changes from parent.""" + source = _make_repo(tmp_path, "source") + clone_dir = tmp_path / "clone" + + run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") + + setup_mind_branch_and_parent(clone_dir, "selene", str(source)) + + # Make a change in the source repo + (source / "new_file.txt").write_text("new content") + add_and_commit_git_repo(source, tmp_path, message="add new file") + + # Read the parent info and merge + parent_info = read_parent_info(clone_dir) + new_hash = fetch_and_merge_parent(clone_dir, parent_info) + + # Verify the new file is present in the clone + assert (clone_dir / "new_file.txt").exists() + assert (clone_dir / "new_file.txt").read_text() == "new content" + + # Verify the parent hash was updated + updated_parent = read_parent_info(clone_dir) + assert updated_parent.hash == new_hash + assert updated_parent.hash != parent_info.hash + + +def test_fetch_and_merge_parent_noop_when_up_to_date(tmp_path: Path) -> None: + """Verify fetch_and_merge_parent succeeds when already up to date.""" + source = _make_repo(tmp_path, "source") + clone_dir = tmp_path / "clone" + + run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") + setup_mind_branch_and_parent(clone_dir, "selene", str(source)) + + parent_info = read_parent_info(clone_dir) + new_hash = fetch_and_merge_parent(clone_dir, parent_info) + + # Hash should be the same since no changes were made + assert new_hash == parent_info.hash diff --git a/apps/minds/imbue/minds/forwarding_server/vendor_mng.py b/apps/minds/imbue/minds/forwarding_server/vendor_mng.py index 4e637aae7..d1994cf74 100644 --- a/apps/minds/imbue/minds/forwarding_server/vendor_mng.py +++ b/apps/minds/imbue/minds/forwarding_server/vendor_mng.py @@ -71,7 +71,7 @@ def default_vendor_configs(mng_repo_root: Path | None) -> tuple[VendorRepoConfig _VENDOR_GIT_USER_EMAIL: Final[str] = "minds@localhost" -def _ensure_git_identity(repo_dir: Path) -> None: +def ensure_git_identity(repo_dir: Path) -> None: """Ensure git user.name and user.email are configured in the repo. ``git subtree add`` creates merge commits, which require a committer @@ -87,12 +87,12 @@ def _ensure_git_identity(repo_dir: Path) -> None: is_checked_after=False, ) if name_result.returncode != 0: - _run_git( + run_git( ["config", "user.name", _VENDOR_GIT_USER_NAME], cwd=repo_dir, error_message="Failed to set git user.name", ) - _run_git( + run_git( ["config", "user.email", _VENDOR_GIT_USER_EMAIL], cwd=repo_dir, error_message="Failed to set git user.email", @@ -110,7 +110,7 @@ def vendor_repos( Raises DirtyRepoError if a local repo has uncommitted or untracked changes. Raises VendorError if any git operation fails. """ - _ensure_git_identity(mind_dir) + ensure_git_identity(mind_dir) for config in configs: vendor_subdir = mind_dir / VENDOR_DIR_NAME / config.name if vendor_subdir.exists(): @@ -135,7 +135,7 @@ def check_repo_is_clean(repo_path: Path) -> None: Raises DirtyRepoError if the working tree is not clean. """ - status_output = _run_git( + status_output = run_git( ["status", "--porcelain"], cwd=repo_path, error_message="Failed to check git status of {}".format(repo_path), @@ -163,7 +163,7 @@ def _resolve_ref_local(repo_path: Path, ref: str | None) -> str: """Resolve the git ref for a local repo, defaulting to HEAD.""" if ref is not None: return ref - return _run_git( + return run_git( ["rev-parse", "HEAD"], cwd=repo_path, error_message="Failed to resolve HEAD of {}".format(repo_path), @@ -185,7 +185,7 @@ def _resolve_ref_remote( """Resolve the git ref for a remote repo, defaulting to HEAD.""" if ref is not None: return ref - ls_output = _run_git( + ls_output = run_git( ["ls-remote", url, "HEAD"], cwd=Path.cwd(), on_output=on_output, @@ -206,7 +206,7 @@ def _add_subtree( ) -> None: """Run ``git subtree add`` to add a repository under vendor//.""" prefix = "{}/{}".format(VENDOR_DIR_NAME, name) - _run_git( + run_git( ["subtree", "add", "--prefix", prefix, url_or_path, ref, "--squash"], cwd=mind_dir, on_output=on_output, @@ -214,7 +214,62 @@ def _add_subtree( ) -def _run_git( +def pull_subtree( + mind_dir: Path, + name: str, + url_or_path: str, + ref: str, + on_output: Callable[[str, bool], None] | None = None, +) -> None: + """Run ``git subtree pull`` to update a repository under vendor//. + + Merges the latest changes from the source repository into the existing + subtree. The subtree must already exist (i.e. was previously added via + ``_add_subtree``). Raises VendorError if the pull fails. + """ + prefix = "{}/{}".format(VENDOR_DIR_NAME, name) + run_git( + ["subtree", "pull", "--prefix", prefix, url_or_path, ref, "--squash"], + cwd=mind_dir, + on_output=on_output, + error_message="Failed to pull git subtree for {}".format(name), + ) + + +def update_vendor_repos( + mind_dir: Path, + configs: tuple[VendorRepoConfig, ...], + on_output: Callable[[str, bool], None] | None = None, +) -> None: + """Pull the latest changes for each vendored git subtree. + + Only updates subtrees whose ``vendor/`` directory already exists. + Subtrees that were never added are skipped (use ``vendor_repos`` first). + + Raises DirtyRepoError if a local repo has uncommitted or untracked changes. + Raises VendorError if any git operation fails. + """ + ensure_git_identity(mind_dir) + for config in configs: + vendor_subdir = mind_dir / VENDOR_DIR_NAME / config.name + if not vendor_subdir.exists(): + logger.debug("vendor/{} does not exist, skipping update", config.name) + continue + + if config.is_local: + repo_path = _resolve_local_path(config.path) + check_repo_is_clean(repo_path) + ref = _resolve_ref_local(repo_path, config.ref) + logger.debug("Updating vendor/{} from local repo {} at {}", config.name, repo_path, ref) + pull_subtree(mind_dir, config.name, str(repo_path), ref, on_output) + else: + url = _require_url(config.url) + ref = _resolve_ref_remote(url, config.ref, on_output) + logger.debug("Updating vendor/{} from {} at {}", config.name, url, ref) + pull_subtree(mind_dir, config.name, url, ref, on_output) + + +def run_git( args: list[str], cwd: Path, on_output: Callable[[str, bool], None] | None = None, diff --git a/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py b/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py index 62f3349c7..876923131 100644 --- a/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py +++ b/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py @@ -10,6 +10,7 @@ from imbue.minds.forwarding_server.vendor_mng import check_repo_is_clean from imbue.minds.forwarding_server.vendor_mng import default_vendor_configs from imbue.minds.forwarding_server.vendor_mng import find_mng_repo_root +from imbue.minds.forwarding_server.vendor_mng import update_vendor_repos from imbue.minds.forwarding_server.vendor_mng import vendor_repos from imbue.minds.testing import add_and_commit_git_repo from imbue.minds.testing import init_and_commit_git_repo @@ -211,3 +212,54 @@ def test_vendor_repos_remote_adds_subtree(tmp_path: Path) -> None: vendor_subdir = mind_dir / VENDOR_DIR_NAME / "remote-lib" assert vendor_subdir.is_dir() assert (vendor_subdir / "hello.txt").read_text() == "hello" + + +# -- update_vendor_repos tests -- + + +def test_update_vendor_repos_pulls_local_changes(tmp_path: Path) -> None: + """update_vendor_repos pulls new changes from a local source repo.""" + source = _make_source_repo(tmp_path) + mind_dir = _make_mind_repo(tmp_path) + + config = VendorRepoConfig(name=NonEmptyStr("my-lib"), path=str(source)) + vendor_repos(mind_dir, (config,)) + + # Make a change in the source repo + (source / "hello.txt").write_text("updated") + add_and_commit_git_repo(source, tmp_path, message="update hello") + + # Update the subtree + update_vendor_repos(mind_dir, (config,)) + + assert (mind_dir / VENDOR_DIR_NAME / "my-lib" / "hello.txt").read_text() == "updated" + + +def test_update_vendor_repos_skips_missing_subtree(tmp_path: Path) -> None: + """update_vendor_repos skips configs whose vendor directory does not exist.""" + mind_dir = _make_mind_repo(tmp_path) + source = _make_source_repo(tmp_path) + + config = VendorRepoConfig(name=NonEmptyStr("not-vendored"), path=str(source)) + # Should not raise -- just skip the non-existent subtree + update_vendor_repos(mind_dir, (config,)) + + assert not (mind_dir / VENDOR_DIR_NAME / "not-vendored").exists() + + +def test_update_vendor_repos_remote_pulls_changes(tmp_path: Path) -> None: + """update_vendor_repos pulls changes from a remote repo (using local path as URL).""" + source = _make_source_repo(tmp_path) + mind_dir = _make_mind_repo(tmp_path) + + config = VendorRepoConfig(name=NonEmptyStr("remote-lib"), url=str(source)) + vendor_repos(mind_dir, (config,)) + + # Make a change in the source repo + (source / "new_file.txt").write_text("new content") + add_and_commit_git_repo(source, tmp_path, message="add new file") + + # Update the subtree + update_vendor_repos(mind_dir, (config,)) + + assert (mind_dir / VENDOR_DIR_NAME / "remote-lib" / "new_file.txt").read_text() == "new content" diff --git a/apps/minds/imbue/minds/main.py b/apps/minds/imbue/minds/main.py index dfe94b916..5627c56ff 100644 --- a/apps/minds/imbue/minds/main.py +++ b/apps/minds/imbue/minds/main.py @@ -1,6 +1,7 @@ import click from imbue.minds.cli.forward import forward +from imbue.minds.cli.update import update from imbue.minds.utils.logging import console_level_from_verbose_and_quiet from imbue.minds.utils.logging import setup_logging @@ -18,3 +19,4 @@ def cli(ctx: click.Context, verbose: int, quiet: bool) -> None: cli.add_command(forward) +cli.add_command(update) From 2fd8e6fec7b1e65a54793235c3633f3feb7edab6 Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:04:53 -0700 Subject: [PATCH 2/8] Fix reviewer issues: use domain types, proper data model, shared test helper - Use GitUrl, GitBranch, and new GitCommitHash primitives in ParentInfo instead of bare str fields (style guide compliance) - Create MindAgentRecord FrozenModel for find_mind_agent return type instead of raw dict[str, object] - Extract shared make_git_repo helper to forwarding_server conftest.py to eliminate duplication between parent_tracking_test and vendor_mng_test - Find agents by name (--include 'name == "..."') instead of by label - Deduplicate run_mng_stop/run_mng_start into _run_mng_command helper - Remove unused _MNG_COMMAND_TIMEOUT_SECONDS constant - Add logger.trace for swallowed JSONDecodeError in _parse_agents_from_output - Fix pull_subtree docstring reference to private function Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/settings.json | 3 - apps/minds/imbue/minds/cli/update.py | 98 ++++++++++--------- .../imbue/minds/forwarding_server/conftest.py | 13 +++ .../forwarding_server/parent_tracking.py | 63 +++++++----- .../forwarding_server/parent_tracking_test.py | 42 ++++---- .../minds/forwarding_server/vendor_mng.py | 2 +- .../forwarding_server/vendor_mng_test.py | 36 +++---- apps/minds/imbue/minds/primitives.py | 6 ++ 8 files changed, 141 insertions(+), 122 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 4e066c25a..43a88656a 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,7 +1,4 @@ { - "env": { - "CLAUDE_CODE_MAX_OUTPUT_TOKENS": "64000" - }, "hooks": { "SessionStart": [ { diff --git a/apps/minds/imbue/minds/cli/update.py b/apps/minds/imbue/minds/cli/update.py index 2284e34f2..eda68a9d4 100644 --- a/apps/minds/imbue/minds/cli/update.py +++ b/apps/minds/imbue/minds/cli/update.py @@ -10,12 +10,13 @@ import json from pathlib import Path -from typing import Final import click from loguru import logger +from pydantic import Field from imbue.concurrency_group.concurrency_group import ConcurrencyGroup +from imbue.imbue_common.frozen_model import FrozenModel from imbue.minds.config.data_types import MNG_BINARY from imbue.minds.errors import MindError from imbue.minds.errors import MngCommandError @@ -27,21 +28,37 @@ from imbue.minds.forwarding_server.vendor_mng import update_vendor_repos from imbue.mng.primitives import AgentId -_MNG_COMMAND_TIMEOUT_SECONDS: Final[float] = 120.0 +class MindAgentRecord(FrozenModel): + """Essential fields from a mind agent's ``mng list`` JSON record. -def find_mind_agent(agent_name: str) -> dict[str, object]: + Validated on construction so callers get a clear error if required + fields are missing from the mng output. + """ + + agent_id: AgentId = Field(description="The agent's unique identifier") + work_dir: Path = Field(description="Absolute path to the agent's working directory") + + +def find_mind_agent(agent_name: str) -> MindAgentRecord: """Find a mind agent by name using ``mng list``. - Searches for agents with the label ``mind=``. - Returns the full agent record dict from the JSON output. + Searches for agents whose name matches ``agent_name`` (the agent name + is set to the mind name during creation, so each mind has a unique name). + Returns a validated MindAgentRecord with the agent's ID and work directory. - Raises MindError if the agent cannot be found. + Raises MindError if the agent cannot be found or the record is malformed. """ cg = ConcurrencyGroup(name="mng-list") with cg: result = cg.run_process_to_completion( - command=[MNG_BINARY, "list", "--label", "mind={}".format(agent_name), "--format=json"], + command=[ + MNG_BINARY, + "list", + "--include", + 'name == "{}"'.format(agent_name), + "--format=json", + ], is_checked_after=False, ) if result.returncode != 0: @@ -55,7 +72,17 @@ def find_mind_agent(agent_name: str) -> dict[str, object]: if not agents: raise MindError("No mind found with name '{}'".format(agent_name)) - return agents[0] + raw = agents[0] + raw_id = raw.get("id") + raw_work_dir = raw.get("work_dir") + if raw_id is None or raw_work_dir is None: + raise MindError( + "Agent record for '{}' is missing required fields (id={}, work_dir={})".format( + agent_name, raw_id, raw_work_dir + ) + ) + + return MindAgentRecord(agent_id=AgentId(str(raw_id)), work_dir=Path(str(raw_work_dir))) def _parse_agents_from_output(stdout: str) -> list[dict[str, object]]: @@ -71,46 +98,27 @@ def _parse_agents_from_output(stdout: str) -> list[dict[str, object]]: data = json.loads(stripped) return list(data.get("agents", [])) except json.JSONDecodeError: + logger.trace("Failed to parse JSON from mng list output line: {}", stripped[:200]) continue return [] -def run_mng_stop(agent_id: AgentId) -> None: - """Stop a mind agent via ``mng stop``. - - Raises MngCommandError if the command fails. - """ - logger.info("Stopping agent {}...", agent_id) - cg = ConcurrencyGroup(name="mng-stop") - with cg: - result = cg.run_process_to_completion( - command=[MNG_BINARY, "stop", str(agent_id)], - is_checked_after=False, - ) - if result.returncode != 0: - raise MngCommandError( - "mng stop failed (exit code {}):\n{}".format( - result.returncode, - result.stderr.strip() if result.stderr.strip() else result.stdout.strip(), - ) - ) - - -def run_mng_start(agent_id: AgentId) -> None: - """Start a mind agent via ``mng start``. +def _run_mng_command(verb: str, agent_id: AgentId) -> None: + """Run an ``mng `` command. Raises MngCommandError if the command fails. """ - logger.info("Starting agent {}...", agent_id) - cg = ConcurrencyGroup(name="mng-start") + logger.info("Running mng {} {}...", verb, agent_id) + cg = ConcurrencyGroup(name="mng-{}".format(verb)) with cg: result = cg.run_process_to_completion( - command=[MNG_BINARY, "start", str(agent_id)], + command=[MNG_BINARY, verb, str(agent_id)], is_checked_after=False, ) if result.returncode != 0: raise MngCommandError( - "mng start failed (exit code {}):\n{}".format( + "mng {} failed (exit code {}):\n{}".format( + verb, result.returncode, result.stderr.strip() if result.stderr.strip() else result.stdout.strip(), ) @@ -126,26 +134,24 @@ def update(agent_name: str) -> None: subtrees, and starts the mind back up. """ logger.info("Looking up mind '{}'...", agent_name) - agent_record = find_mind_agent(agent_name) - agent_id = AgentId(str(agent_record["id"])) - work_dir = Path(str(agent_record["work_dir"])) + record = find_mind_agent(agent_name) - logger.info("Found mind '{}' (agent_id={}, work_dir={})", agent_name, agent_id, work_dir) + logger.info("Found mind '{}' (agent_id={}, work_dir={})", agent_name, record.agent_id, record.work_dir) - run_mng_stop(agent_id) + _run_mng_command("stop", record.agent_id) logger.info("Merging latest code from parent repository...") - parent_info = read_parent_info(work_dir) - new_hash = fetch_and_merge_parent(work_dir, parent_info) - logger.info("Merged parent changes (new hash: {})", new_hash[:12]) + parent_info = read_parent_info(record.work_dir) + new_hash = fetch_and_merge_parent(record.work_dir, parent_info) + logger.info("Merged parent changes (new hash: {})", str(new_hash)[:12]) logger.info("Updating vendored subtrees...") - settings = load_creation_settings(work_dir) + settings = load_creation_settings(record.work_dir) mng_repo_root = find_mng_repo_root() vendor_configs = settings.vendor if settings.vendor else default_vendor_configs(mng_repo_root) - update_vendor_repos(work_dir, vendor_configs) + update_vendor_repos(record.work_dir, vendor_configs) logger.info("Vendored subtrees updated ({} configured)", len(vendor_configs)) - run_mng_start(agent_id) + _run_mng_command("start", record.agent_id) logger.info("Mind '{}' updated successfully.", agent_name) diff --git a/apps/minds/imbue/minds/forwarding_server/conftest.py b/apps/minds/imbue/minds/forwarding_server/conftest.py index 907ecb44f..cfc314295 100644 --- a/apps/minds/imbue/minds/forwarding_server/conftest.py +++ b/apps/minds/imbue/minds/forwarding_server/conftest.py @@ -10,6 +10,7 @@ from imbue.minds.forwarding_server.backend_resolver import parse_agents_from_json from imbue.minds.forwarding_server.backend_resolver import parse_server_log_records from imbue.minds.primitives import ServerName +from imbue.minds.testing import init_and_commit_git_repo from imbue.mng.primitives import AgentId from imbue.mng.primitives import AgentName from imbue.mng.primitives import DiscoveredAgent @@ -86,3 +87,15 @@ def make_resolver_with_data( resolver.update_servers(AgentId(agent_id_str), servers) return resolver + + +def make_git_repo(tmp_path: Path, name: str = "repo") -> Path: + """Create a minimal git repo with a committed file. + + Shared helper for tests that need a local git repo to operate on. + """ + repo = tmp_path / name + repo.mkdir() + (repo / "hello.txt").write_text("hello") + init_and_commit_git_repo(repo, tmp_path) + return repo diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking.py index c70cab3af..cf3b15b5c 100644 --- a/apps/minds/imbue/minds/forwarding_server/parent_tracking.py +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking.py @@ -25,6 +25,9 @@ from imbue.imbue_common.frozen_model import FrozenModel from imbue.minds.forwarding_server.vendor_mng import ensure_git_identity from imbue.minds.forwarding_server.vendor_mng import run_git +from imbue.minds.primitives import GitBranch +from imbue.minds.primitives import GitCommitHash +from imbue.minds.primitives import GitUrl PARENT_FILE_NAME: Final[str] = ".parent" @@ -38,30 +41,34 @@ class ParentInfo(FrozenModel): cloned to create the mind. """ - url: str = Field(description="Git URL of the parent repository") - branch: str = Field(description="Branch name in the parent repository") - hash: str = Field(description="Commit hash at the time of creation or last update") + url: GitUrl = Field(description="Git URL of the parent repository") + branch: GitBranch = Field(description="Branch name in the parent repository") + hash: GitCommitHash = Field(description="Commit hash at the time of creation or last update") -def get_current_branch(repo_dir: Path) -> str: +def get_current_branch(repo_dir: Path) -> GitBranch: """Return the current branch name of a git repository. Returns ``HEAD`` if the repository is in detached HEAD state. """ - return run_git( - ["rev-parse", "--abbrev-ref", "HEAD"], - cwd=repo_dir, - error_message="Failed to get current branch of {}".format(repo_dir), - ).strip() + return GitBranch( + run_git( + ["rev-parse", "--abbrev-ref", "HEAD"], + cwd=repo_dir, + error_message="Failed to get current branch of {}".format(repo_dir), + ).strip() + ) -def get_current_commit_hash(repo_dir: Path) -> str: +def get_current_commit_hash(repo_dir: Path) -> GitCommitHash: """Return the full commit hash of HEAD in a git repository.""" - return run_git( - ["rev-parse", "HEAD"], - cwd=repo_dir, - error_message="Failed to get current commit hash of {}".format(repo_dir), - ).strip() + return GitCommitHash( + run_git( + ["rev-parse", "HEAD"], + cwd=repo_dir, + error_message="Failed to get current commit hash of {}".format(repo_dir), + ).strip() + ) def checkout_mind_branch( @@ -103,19 +110,19 @@ def write_parent_info( ) run_git( - ["config", "--file", parent_file, "parent.url", parent_info.url], + ["config", "--file", parent_file, "parent.url", str(parent_info.url)], cwd=repo_dir, on_output=on_output, error_message="Failed to write parent.url to {}".format(parent_file), ) run_git( - ["config", "--file", parent_file, "parent.branch", parent_info.branch], + ["config", "--file", parent_file, "parent.branch", str(parent_info.branch)], cwd=repo_dir, on_output=on_output, error_message="Failed to write parent.branch to {}".format(parent_file), ) run_git( - ["config", "--file", parent_file, "parent.hash", parent_info.hash], + ["config", "--file", parent_file, "parent.hash", str(parent_info.hash)], cwd=repo_dir, on_output=on_output, error_message="Failed to write parent.hash to {}".format(parent_file), @@ -145,7 +152,7 @@ def read_parent_info(repo_dir: Path) -> ParentInfo: error_message="Failed to read parent.hash from {}".format(parent_file), ).strip() - return ParentInfo(url=url, branch=branch, hash=hash_value) + return ParentInfo(url=GitUrl(url), branch=GitBranch(branch), hash=GitCommitHash(hash_value)) def commit_parent_file( @@ -206,7 +213,7 @@ def setup_mind_branch_and_parent( checkout_mind_branch(repo_dir, mind_name, on_output) - parent_info = ParentInfo(url=git_url, branch=parent_branch, hash=parent_hash) + parent_info = ParentInfo(url=GitUrl(git_url), branch=parent_branch, hash=parent_hash) write_parent_info(repo_dir, parent_info, on_output) commit_parent_file(repo_dir, on_output) @@ -215,7 +222,7 @@ def fetch_and_merge_parent( repo_dir: Path, parent_info: ParentInfo, on_output: Callable[[str, bool], None] | None = None, -) -> str: +) -> GitCommitHash: """Fetch the latest code from the parent repository and merge it. Fetches the parent branch and merges FETCH_HEAD into the current branch. @@ -230,17 +237,19 @@ def fetch_and_merge_parent( logger.debug("Fetching from {} branch {}", parent_info.url, parent_info.branch) run_git( - ["fetch", parent_info.url, parent_info.branch], + ["fetch", str(parent_info.url), str(parent_info.branch)], cwd=repo_dir, on_output=on_output, error_message="Failed to fetch from {} branch {}".format(parent_info.url, parent_info.branch), ) - new_hash = run_git( - ["rev-parse", "FETCH_HEAD"], - cwd=repo_dir, - error_message="Failed to resolve FETCH_HEAD", - ).strip() + new_hash = GitCommitHash( + run_git( + ["rev-parse", "FETCH_HEAD"], + cwd=repo_dir, + error_message="Failed to resolve FETCH_HEAD", + ).strip() + ) logger.debug("Merging FETCH_HEAD ({})", new_hash) run_git( diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py index e4dcddefb..edd96f358 100644 --- a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py @@ -3,6 +3,7 @@ import pytest from imbue.minds.errors import VendorError +from imbue.minds.forwarding_server.conftest import make_git_repo from imbue.minds.forwarding_server.parent_tracking import MIND_BRANCH_PREFIX from imbue.minds.forwarding_server.parent_tracking import PARENT_FILE_NAME from imbue.minds.forwarding_server.parent_tracking import ParentInfo @@ -15,46 +16,39 @@ from imbue.minds.forwarding_server.parent_tracking import setup_mind_branch_and_parent from imbue.minds.forwarding_server.parent_tracking import write_parent_info from imbue.minds.forwarding_server.vendor_mng import run_git +from imbue.minds.primitives import GitBranch +from imbue.minds.primitives import GitCommitHash +from imbue.minds.primitives import GitUrl from imbue.minds.testing import add_and_commit_git_repo -from imbue.minds.testing import init_and_commit_git_repo - - -def _make_repo(tmp_path: Path, name: str = "repo") -> Path: - """Create a minimal git repo with a committed file.""" - repo = tmp_path / name - repo.mkdir() - (repo / "hello.txt").write_text("hello") - init_and_commit_git_repo(repo, tmp_path) - return repo def test_get_current_branch_returns_branch_name(tmp_path: Path) -> None: - repo = _make_repo(tmp_path) + repo = make_git_repo(tmp_path) branch = get_current_branch(repo) # Default branch is typically "master" or "main" depending on git config assert branch in ("master", "main") def test_get_current_commit_hash_returns_full_hash(tmp_path: Path) -> None: - repo = _make_repo(tmp_path) + repo = make_git_repo(tmp_path) commit_hash = get_current_commit_hash(repo) assert len(commit_hash) == 40 assert all(c in "0123456789abcdef" for c in commit_hash) def test_checkout_mind_branch_creates_new_branch(tmp_path: Path) -> None: - repo = _make_repo(tmp_path) + repo = make_git_repo(tmp_path) checkout_mind_branch(repo, "selene") branch = get_current_branch(repo) assert branch == "{}selene".format(MIND_BRANCH_PREFIX) def test_write_and_read_parent_info_roundtrips(tmp_path: Path) -> None: - repo = _make_repo(tmp_path) + repo = make_git_repo(tmp_path) parent = ParentInfo( - url="https://github.com/org/repo.git", - branch="main", - hash="abc123def456" * 3 + "abcd", + url=GitUrl("https://github.com/org/repo.git"), + branch=GitBranch("main"), + hash=GitCommitHash("abc123def456" * 3 + "abcd"), ) write_parent_info(repo, parent) result = read_parent_info(repo) @@ -62,14 +56,16 @@ def test_write_and_read_parent_info_roundtrips(tmp_path: Path) -> None: def test_read_parent_info_raises_when_file_missing(tmp_path: Path) -> None: - repo = _make_repo(tmp_path) + repo = make_git_repo(tmp_path) with pytest.raises(VendorError, match="Failed to read parent.url"): read_parent_info(repo) def test_commit_parent_file_creates_commit(tmp_path: Path) -> None: - repo = _make_repo(tmp_path) - parent = ParentInfo(url="https://example.com/repo.git", branch="main", hash="a" * 40) + repo = make_git_repo(tmp_path) + parent = ParentInfo( + url=GitUrl("https://example.com/repo.git"), branch=GitBranch("main"), hash=GitCommitHash("a" * 40) + ) write_parent_info(repo, parent) commit_parent_file(repo) @@ -83,7 +79,7 @@ def test_commit_parent_file_creates_commit(tmp_path: Path) -> None: def test_setup_mind_branch_and_parent_full_flow(tmp_path: Path) -> None: """Verify the full setup flow: branch creation, parent tracking, and commit.""" - source = _make_repo(tmp_path, "source") + source = make_git_repo(tmp_path, "source") clone_dir = tmp_path / "clone" run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") @@ -106,7 +102,7 @@ def test_setup_mind_branch_and_parent_full_flow(tmp_path: Path) -> None: def test_fetch_and_merge_parent_merges_changes(tmp_path: Path) -> None: """Verify that fetch_and_merge_parent pulls new changes from parent.""" - source = _make_repo(tmp_path, "source") + source = make_git_repo(tmp_path, "source") clone_dir = tmp_path / "clone" run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") @@ -133,7 +129,7 @@ def test_fetch_and_merge_parent_merges_changes(tmp_path: Path) -> None: def test_fetch_and_merge_parent_noop_when_up_to_date(tmp_path: Path) -> None: """Verify fetch_and_merge_parent succeeds when already up to date.""" - source = _make_repo(tmp_path, "source") + source = make_git_repo(tmp_path, "source") clone_dir = tmp_path / "clone" run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") diff --git a/apps/minds/imbue/minds/forwarding_server/vendor_mng.py b/apps/minds/imbue/minds/forwarding_server/vendor_mng.py index d1994cf74..0ded7e23e 100644 --- a/apps/minds/imbue/minds/forwarding_server/vendor_mng.py +++ b/apps/minds/imbue/minds/forwarding_server/vendor_mng.py @@ -225,7 +225,7 @@ def pull_subtree( Merges the latest changes from the source repository into the existing subtree. The subtree must already exist (i.e. was previously added via - ``_add_subtree``). Raises VendorError if the pull fails. + ``vendor_repos``). Raises VendorError if the pull fails. """ prefix = "{}/{}".format(VENDOR_DIR_NAME, name) run_git( diff --git a/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py b/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py index 876923131..372931bc6 100644 --- a/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py +++ b/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py @@ -6,6 +6,7 @@ from imbue.imbue_common.primitives import NonEmptyStr from imbue.minds.errors import DirtyRepoError from imbue.minds.errors import VendorError +from imbue.minds.forwarding_server.conftest import make_git_repo from imbue.minds.forwarding_server.vendor_mng import VENDOR_DIR_NAME from imbue.minds.forwarding_server.vendor_mng import check_repo_is_clean from imbue.minds.forwarding_server.vendor_mng import default_vendor_configs @@ -25,15 +26,6 @@ def _make_mind_repo(tmp_path: Path) -> Path: return mind_dir -def _make_source_repo(tmp_path: Path, name: str = "source") -> Path: - """Create a source git repo with a committed file.""" - source = tmp_path / name - source.mkdir() - (source / "hello.txt").write_text("hello") - init_and_commit_git_repo(source, tmp_path) - return source - - def test_find_mng_repo_root_returns_path() -> None: """When running from within the mng monorepo, finds the root.""" root = find_mng_repo_root() @@ -57,7 +49,7 @@ def test_vendor_repos_skips_when_vendor_dir_exists(tmp_path: Path) -> None: def test_vendor_repos_local_adds_subtree(tmp_path: Path) -> None: """A local repo is added as a git subtree under vendor//.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") mind_dir = _make_mind_repo(tmp_path) config = VendorRepoConfig(name=NonEmptyStr("my-lib"), path=str(source)) @@ -70,7 +62,7 @@ def test_vendor_repos_local_adds_subtree(tmp_path: Path) -> None: def test_vendor_repos_local_creates_commit(tmp_path: Path) -> None: """Subtree addition creates a merge commit in the mind repo.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") mind_dir = _make_mind_repo(tmp_path) config = VendorRepoConfig(name=NonEmptyStr("my-lib"), path=str(source)) @@ -87,7 +79,7 @@ def test_vendor_repos_local_creates_commit(tmp_path: Path) -> None: def test_vendor_repos_local_at_specific_ref(tmp_path: Path) -> None: """When ref is specified, that exact commit is vendored.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") first_hash = subprocess.run( ["git", "rev-parse", "HEAD"], @@ -108,7 +100,7 @@ def test_vendor_repos_local_at_specific_ref(tmp_path: Path) -> None: def test_vendor_repos_local_dirty_repo_raises(tmp_path: Path) -> None: """Raises DirtyRepoError when the local repo has uncommitted changes.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") (source / "hello.txt").write_text("modified") mind_dir = _make_mind_repo(tmp_path) @@ -120,7 +112,7 @@ def test_vendor_repos_local_dirty_repo_raises(tmp_path: Path) -> None: def test_vendor_repos_local_untracked_files_raises(tmp_path: Path) -> None: """Raises DirtyRepoError when the local repo has untracked files.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") (source / "new_file.txt").write_text("untracked") mind_dir = _make_mind_repo(tmp_path) @@ -132,7 +124,7 @@ def test_vendor_repos_local_untracked_files_raises(tmp_path: Path) -> None: def test_vendor_repos_multiple_repos(tmp_path: Path) -> None: """Multiple repos can be vendored into separate directories.""" - source_a = _make_source_repo(tmp_path, name="repo-a") + source_a = make_git_repo(tmp_path, "repo-a") source_b = tmp_path / "repo-b" source_b.mkdir() (source_b / "data.txt").write_text("data") @@ -160,13 +152,13 @@ def test_vendor_repos_invalid_local_path_raises(tmp_path: Path) -> None: def test_check_repo_is_clean_passes_for_clean_repo(tmp_path: Path) -> None: """check_repo_is_clean does not raise for a clean repo.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") check_repo_is_clean(source) def test_check_repo_is_clean_raises_for_modified_file(tmp_path: Path) -> None: """check_repo_is_clean raises when a tracked file is modified.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") (source / "hello.txt").write_text("changed") with pytest.raises(DirtyRepoError): @@ -175,7 +167,7 @@ def test_check_repo_is_clean_raises_for_modified_file(tmp_path: Path) -> None: def test_check_repo_is_clean_raises_for_untracked_file(tmp_path: Path) -> None: """check_repo_is_clean raises when untracked files exist.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") (source / "new.txt").write_text("new") with pytest.raises(DirtyRepoError): @@ -203,7 +195,7 @@ def test_default_vendor_configs_production_mode() -> None: def test_vendor_repos_remote_adds_subtree(tmp_path: Path) -> None: """A remote repo (using a local path as the URL) is added as a subtree.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") mind_dir = _make_mind_repo(tmp_path) config = VendorRepoConfig(name=NonEmptyStr("remote-lib"), url=str(source)) @@ -219,7 +211,7 @@ def test_vendor_repos_remote_adds_subtree(tmp_path: Path) -> None: def test_update_vendor_repos_pulls_local_changes(tmp_path: Path) -> None: """update_vendor_repos pulls new changes from a local source repo.""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") mind_dir = _make_mind_repo(tmp_path) config = VendorRepoConfig(name=NonEmptyStr("my-lib"), path=str(source)) @@ -238,7 +230,7 @@ def test_update_vendor_repos_pulls_local_changes(tmp_path: Path) -> None: def test_update_vendor_repos_skips_missing_subtree(tmp_path: Path) -> None: """update_vendor_repos skips configs whose vendor directory does not exist.""" mind_dir = _make_mind_repo(tmp_path) - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") config = VendorRepoConfig(name=NonEmptyStr("not-vendored"), path=str(source)) # Should not raise -- just skip the non-existent subtree @@ -249,7 +241,7 @@ def test_update_vendor_repos_skips_missing_subtree(tmp_path: Path) -> None: def test_update_vendor_repos_remote_pulls_changes(tmp_path: Path) -> None: """update_vendor_repos pulls changes from a remote repo (using local path as URL).""" - source = _make_source_repo(tmp_path) + source = make_git_repo(tmp_path, "source") mind_dir = _make_mind_repo(tmp_path) config = VendorRepoConfig(name=NonEmptyStr("remote-lib"), url=str(source)) diff --git a/apps/minds/imbue/minds/primitives.py b/apps/minds/imbue/minds/primitives.py index bc001108c..aea9feefb 100644 --- a/apps/minds/imbue/minds/primitives.py +++ b/apps/minds/imbue/minds/primitives.py @@ -37,3 +37,9 @@ class GitBranch(NonEmptyStr): """A git branch name to clone.""" ... + + +class GitCommitHash(NonEmptyStr): + """A full git commit hash (40 hex characters).""" + + ... From 5d0da9dfb54c84dec6a8b1ff10d931ca2da452a2 Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:09:26 -0700 Subject: [PATCH 3/8] Fix reviewer round 2: deduplicate JSON parser, move helper to testing.py - Rename _parse_agents_from_output to parse_agents_from_mng_output (public) and have testing.py's parse_mng_list_json delegate to it, eliminating the duplicate JSON parsing logic - Move make_git_repo from forwarding_server/conftest.py to testing.py per project convention (conftest.py is for fixtures, testing.py is for explicitly imported helper functions) Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/minds/imbue/minds/cli/update.py | 4 +-- apps/minds/imbue/minds/cli/update_test.py | 26 +++++++-------- .../imbue/minds/forwarding_server/conftest.py | 13 -------- .../forwarding_server/parent_tracking_test.py | 2 +- .../forwarding_server/vendor_mng_test.py | 2 +- apps/minds/imbue/minds/testing.py | 32 +++++++++++-------- 6 files changed, 35 insertions(+), 44 deletions(-) diff --git a/apps/minds/imbue/minds/cli/update.py b/apps/minds/imbue/minds/cli/update.py index eda68a9d4..ba3d10c95 100644 --- a/apps/minds/imbue/minds/cli/update.py +++ b/apps/minds/imbue/minds/cli/update.py @@ -68,7 +68,7 @@ def find_mind_agent(agent_name: str) -> MindAgentRecord: ) ) - agents = _parse_agents_from_output(result.stdout) + agents = parse_agents_from_mng_output(result.stdout) if not agents: raise MindError("No mind found with name '{}'".format(agent_name)) @@ -85,7 +85,7 @@ def find_mind_agent(agent_name: str) -> MindAgentRecord: return MindAgentRecord(agent_id=AgentId(str(raw_id)), work_dir=Path(str(raw_work_dir))) -def _parse_agents_from_output(stdout: str) -> list[dict[str, object]]: +def parse_agents_from_mng_output(stdout: str) -> list[dict[str, object]]: """Parse agent records from ``mng list --format json`` output. Handles the case where stdout may contain non-JSON lines diff --git a/apps/minds/imbue/minds/cli/update_test.py b/apps/minds/imbue/minds/cli/update_test.py index 780388c02..0e13803ea 100644 --- a/apps/minds/imbue/minds/cli/update_test.py +++ b/apps/minds/imbue/minds/cli/update_test.py @@ -1,10 +1,10 @@ import json -from imbue.minds.cli.update import _parse_agents_from_output +from imbue.minds.cli.update import parse_agents_from_mng_output -def test_parse_agents_from_output_extracts_records() -> None: - """Verify _parse_agents_from_output extracts agent records from JSON.""" +def testparse_agents_from_mng_output_extracts_records() -> None: + """Verify parse_agents_from_mng_output extracts agent records from JSON.""" json_str = json.dumps( { "agents": [ @@ -12,28 +12,28 @@ def test_parse_agents_from_output_extracts_records() -> None: ] } ) - agents = _parse_agents_from_output(json_str) + agents = parse_agents_from_mng_output(json_str) assert len(agents) == 1 assert agents[0]["id"] == "agent-abc123" assert agents[0]["name"] == "selene" -def test_parse_agents_from_output_handles_empty() -> None: - """Verify _parse_agents_from_output returns empty list for no agents.""" +def testparse_agents_from_mng_output_handles_empty() -> None: + """Verify parse_agents_from_mng_output returns empty list for no agents.""" json_str = json.dumps({"agents": []}) - agents = _parse_agents_from_output(json_str) + agents = parse_agents_from_mng_output(json_str) assert agents == [] -def test_parse_agents_from_output_handles_non_json() -> None: - """Verify _parse_agents_from_output handles non-JSON output gracefully.""" - agents = _parse_agents_from_output("not json at all") +def testparse_agents_from_mng_output_handles_non_json() -> None: + """Verify parse_agents_from_mng_output handles non-JSON output gracefully.""" + agents = parse_agents_from_mng_output("not json at all") assert agents == [] -def test_parse_agents_from_output_handles_mixed_output() -> None: - """Verify _parse_agents_from_output handles SSH errors mixed with JSON.""" +def testparse_agents_from_mng_output_handles_mixed_output() -> None: + """Verify parse_agents_from_mng_output handles SSH errors mixed with JSON.""" output = "WARNING: some SSH error\n" + json.dumps({"agents": [{"id": "agent-xyz", "name": "test"}]}) - agents = _parse_agents_from_output(output) + agents = parse_agents_from_mng_output(output) assert len(agents) == 1 assert agents[0]["id"] == "agent-xyz" diff --git a/apps/minds/imbue/minds/forwarding_server/conftest.py b/apps/minds/imbue/minds/forwarding_server/conftest.py index cfc314295..907ecb44f 100644 --- a/apps/minds/imbue/minds/forwarding_server/conftest.py +++ b/apps/minds/imbue/minds/forwarding_server/conftest.py @@ -10,7 +10,6 @@ from imbue.minds.forwarding_server.backend_resolver import parse_agents_from_json from imbue.minds.forwarding_server.backend_resolver import parse_server_log_records from imbue.minds.primitives import ServerName -from imbue.minds.testing import init_and_commit_git_repo from imbue.mng.primitives import AgentId from imbue.mng.primitives import AgentName from imbue.mng.primitives import DiscoveredAgent @@ -87,15 +86,3 @@ def make_resolver_with_data( resolver.update_servers(AgentId(agent_id_str), servers) return resolver - - -def make_git_repo(tmp_path: Path, name: str = "repo") -> Path: - """Create a minimal git repo with a committed file. - - Shared helper for tests that need a local git repo to operate on. - """ - repo = tmp_path / name - repo.mkdir() - (repo / "hello.txt").write_text("hello") - init_and_commit_git_repo(repo, tmp_path) - return repo diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py index edd96f358..1f0861708 100644 --- a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py @@ -3,7 +3,6 @@ import pytest from imbue.minds.errors import VendorError -from imbue.minds.forwarding_server.conftest import make_git_repo from imbue.minds.forwarding_server.parent_tracking import MIND_BRANCH_PREFIX from imbue.minds.forwarding_server.parent_tracking import PARENT_FILE_NAME from imbue.minds.forwarding_server.parent_tracking import ParentInfo @@ -20,6 +19,7 @@ from imbue.minds.primitives import GitCommitHash from imbue.minds.primitives import GitUrl from imbue.minds.testing import add_and_commit_git_repo +from imbue.minds.testing import make_git_repo def test_get_current_branch_returns_branch_name(tmp_path: Path) -> None: diff --git a/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py b/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py index 372931bc6..a528e1545 100644 --- a/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py +++ b/apps/minds/imbue/minds/forwarding_server/vendor_mng_test.py @@ -6,7 +6,6 @@ from imbue.imbue_common.primitives import NonEmptyStr from imbue.minds.errors import DirtyRepoError from imbue.minds.errors import VendorError -from imbue.minds.forwarding_server.conftest import make_git_repo from imbue.minds.forwarding_server.vendor_mng import VENDOR_DIR_NAME from imbue.minds.forwarding_server.vendor_mng import check_repo_is_clean from imbue.minds.forwarding_server.vendor_mng import default_vendor_configs @@ -15,6 +14,7 @@ from imbue.minds.forwarding_server.vendor_mng import vendor_repos from imbue.minds.testing import add_and_commit_git_repo from imbue.minds.testing import init_and_commit_git_repo +from imbue.minds.testing import make_git_repo from imbue.mng_claude_mind.data_types import VendorRepoConfig diff --git a/apps/minds/imbue/minds/testing.py b/apps/minds/imbue/minds/testing.py index c63a20f31..8e907ccf0 100644 --- a/apps/minds/imbue/minds/testing.py +++ b/apps/minds/imbue/minds/testing.py @@ -1,4 +1,3 @@ -import json import os import subprocess from pathlib import Path @@ -7,6 +6,7 @@ from loguru import logger from imbue.concurrency_group.concurrency_group import ConcurrencyGroup +from imbue.minds.cli.update import parse_agents_from_mng_output _GIT_TEST_ENV_KEYS: Final[dict[str, str]] = { "GIT_AUTHOR_NAME": "test", @@ -51,6 +51,20 @@ def init_and_commit_git_repo(repo_dir: Path, tmp_path: Path, allow_empty: bool = ) +def make_git_repo(tmp_path: Path, name: str = "repo") -> Path: + """Create a minimal git repo with a committed file. + + Shared helper for tests that need a local git repo to operate on. + Creates a directory under tmp_path with a single ``hello.txt`` file, + initializes a git repo, and commits the file. + """ + repo = tmp_path / name + repo.mkdir() + (repo / "hello.txt").write_text("hello") + init_and_commit_git_repo(repo, tmp_path) + return repo + + def add_and_commit_git_repo(repo_dir: Path, tmp_path: Path, message: str = "update") -> None: """Stage all changes and commit in an existing git repo. @@ -99,20 +113,10 @@ def run_mng(*args: str, timeout: float = 60.0, cwd: Path | None = None) -> subpr def parse_mng_list_json(stdout: str) -> list[dict[str, object]]: """Extract agent records from mng list --format json stdout. - The stdout may contain non-JSON lines (e.g. SSH error tracebacks) - mixed with the JSON. We find the first line starting with '{' and - parse from there. + Delegates to the production implementation in cli.update. Kept here + for backward compatibility with existing test callers. """ - for line in stdout.splitlines(): - stripped = line.strip() - if stripped.startswith("{"): - try: - data = json.loads(stripped) - return list(data.get("agents", [])) - except json.JSONDecodeError: - logger.trace("Failed to parse JSON from mng list output line: {}", stripped[:200]) - continue - return [] + return parse_agents_from_mng_output(stdout) def find_agent(agent_name: str) -> dict[str, object] | None: From 4f46691f377b31ea3f8d4f3e12fdb344d90b4320 Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:12:54 -0700 Subject: [PATCH 4/8] Fix reviewer round 3: fix test naming, move parser to shared module - Fix test function names: test_parse_* (was missing underscore after test) - Move parse_agents_from_mng_output to config/data_types.py (shared module) so neither testing.py nor cli/update.py depends on the other Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/minds/imbue/minds/cli/update.py | 20 +------------------- apps/minds/imbue/minds/cli/update_test.py | 10 +++++----- apps/minds/imbue/minds/config/data_types.py | 21 +++++++++++++++++++++ apps/minds/imbue/minds/testing.py | 2 +- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/apps/minds/imbue/minds/cli/update.py b/apps/minds/imbue/minds/cli/update.py index ba3d10c95..0637a534e 100644 --- a/apps/minds/imbue/minds/cli/update.py +++ b/apps/minds/imbue/minds/cli/update.py @@ -8,7 +8,6 @@ 4. Starts the mind back up (via ``mng start``) """ -import json from pathlib import Path import click @@ -18,6 +17,7 @@ from imbue.concurrency_group.concurrency_group import ConcurrencyGroup from imbue.imbue_common.frozen_model import FrozenModel from imbue.minds.config.data_types import MNG_BINARY +from imbue.minds.config.data_types import parse_agents_from_mng_output from imbue.minds.errors import MindError from imbue.minds.errors import MngCommandError from imbue.minds.forwarding_server.agent_creator import load_creation_settings @@ -85,24 +85,6 @@ def find_mind_agent(agent_name: str) -> MindAgentRecord: return MindAgentRecord(agent_id=AgentId(str(raw_id)), work_dir=Path(str(raw_work_dir))) -def parse_agents_from_mng_output(stdout: str) -> list[dict[str, object]]: - """Parse agent records from ``mng list --format json`` output. - - Handles the case where stdout may contain non-JSON lines - (e.g. SSH error tracebacks) mixed with the JSON output. - """ - for line in stdout.splitlines(): - stripped = line.strip() - if stripped.startswith("{"): - try: - data = json.loads(stripped) - return list(data.get("agents", [])) - except json.JSONDecodeError: - logger.trace("Failed to parse JSON from mng list output line: {}", stripped[:200]) - continue - return [] - - def _run_mng_command(verb: str, agent_id: AgentId) -> None: """Run an ``mng `` command. diff --git a/apps/minds/imbue/minds/cli/update_test.py b/apps/minds/imbue/minds/cli/update_test.py index 0e13803ea..cfb818806 100644 --- a/apps/minds/imbue/minds/cli/update_test.py +++ b/apps/minds/imbue/minds/cli/update_test.py @@ -1,9 +1,9 @@ import json -from imbue.minds.cli.update import parse_agents_from_mng_output +from imbue.minds.config.data_types import parse_agents_from_mng_output -def testparse_agents_from_mng_output_extracts_records() -> None: +def test_parse_agents_from_mng_output_extracts_records() -> None: """Verify parse_agents_from_mng_output extracts agent records from JSON.""" json_str = json.dumps( { @@ -18,20 +18,20 @@ def testparse_agents_from_mng_output_extracts_records() -> None: assert agents[0]["name"] == "selene" -def testparse_agents_from_mng_output_handles_empty() -> None: +def test_parse_agents_from_mng_output_handles_empty() -> None: """Verify parse_agents_from_mng_output returns empty list for no agents.""" json_str = json.dumps({"agents": []}) agents = parse_agents_from_mng_output(json_str) assert agents == [] -def testparse_agents_from_mng_output_handles_non_json() -> None: +def test_parse_agents_from_mng_output_handles_non_json() -> None: """Verify parse_agents_from_mng_output handles non-JSON output gracefully.""" agents = parse_agents_from_mng_output("not json at all") assert agents == [] -def testparse_agents_from_mng_output_handles_mixed_output() -> None: +def test_parse_agents_from_mng_output_handles_mixed_output() -> None: """Verify parse_agents_from_mng_output handles SSH errors mixed with JSON.""" output = "WARNING: some SSH error\n" + json.dumps({"agents": [{"id": "agent-xyz", "name": "test"}]}) agents = parse_agents_from_mng_output(output) diff --git a/apps/minds/imbue/minds/config/data_types.py b/apps/minds/imbue/minds/config/data_types.py index 31d50992d..199e5da46 100644 --- a/apps/minds/imbue/minds/config/data_types.py +++ b/apps/minds/imbue/minds/config/data_types.py @@ -1,6 +1,8 @@ +import json from pathlib import Path from typing import Final +from loguru import logger from pydantic import Field from imbue.imbue_common.frozen_model import FrozenModel @@ -33,3 +35,22 @@ def mind_dir(self, agent_id: AgentId) -> Path: def get_default_data_dir() -> Path: """Return the default data directory for minds (~/.minds).""" return Path.home() / DEFAULT_DATA_DIR_NAME + + +def parse_agents_from_mng_output(stdout: str) -> list[dict[str, object]]: + """Extract agent records from ``mng list --format json`` stdout. + + The stdout may contain non-JSON lines (e.g. SSH error tracebacks) + mixed with the JSON. Finds the first line starting with ``{`` and + parses the ``agents`` array from it. + """ + for line in stdout.splitlines(): + stripped = line.strip() + if stripped.startswith("{"): + try: + data = json.loads(stripped) + return list(data.get("agents", [])) + except json.JSONDecodeError: + logger.trace("Failed to parse JSON from mng list output line: {}", stripped[:200]) + continue + return [] diff --git a/apps/minds/imbue/minds/testing.py b/apps/minds/imbue/minds/testing.py index 8e907ccf0..a35754d56 100644 --- a/apps/minds/imbue/minds/testing.py +++ b/apps/minds/imbue/minds/testing.py @@ -6,7 +6,7 @@ from loguru import logger from imbue.concurrency_group.concurrency_group import ConcurrencyGroup -from imbue.minds.cli.update import parse_agents_from_mng_output +from imbue.minds.config.data_types import parse_agents_from_mng_output _GIT_TEST_ENV_KEYS: Final[dict[str, str]] = { "GIT_AUTHOR_NAME": "test", From 5dc57ff5ba9d02e56e0c64fed7cd0ca33418619d Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:18:33 -0700 Subject: [PATCH 5/8] Fix stale docstring reference in parse_mng_list_json The docstring referenced cli.update but the function was moved to config.data_types. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/minds/imbue/minds/testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/minds/imbue/minds/testing.py b/apps/minds/imbue/minds/testing.py index a35754d56..a8b41265f 100644 --- a/apps/minds/imbue/minds/testing.py +++ b/apps/minds/imbue/minds/testing.py @@ -113,7 +113,7 @@ def run_mng(*args: str, timeout: float = 60.0, cwd: Path | None = None) -> subpr def parse_mng_list_json(stdout: str) -> list[dict[str, object]]: """Extract agent records from mng list --format json stdout. - Delegates to the production implementation in cli.update. Kept here + Delegates to the shared implementation in config.data_types. Kept here for backward compatibility with existing test callers. """ return parse_agents_from_mng_output(stdout) From d0ed1c901855bf8635b20e1d3a7ded5d08de8145 Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:23:12 -0700 Subject: [PATCH 6/8] Move parser tests to data_types_test.py, restore settings env var - Move parse_agents_from_mng_output tests from cli/update_test.py to config/data_types_test.py (where the function lives) - Remove now-empty cli/update_test.py - Restore CLAUDE_CODE_MAX_OUTPUT_TOKENS env var in .claude/settings.json Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/settings.json | 3 ++ apps/minds/imbue/minds/cli/update_test.py | 39 ------------------ .../imbue/minds/config/data_types_test.py | 41 +++++++++++++++++++ 3 files changed, 44 insertions(+), 39 deletions(-) delete mode 100644 apps/minds/imbue/minds/cli/update_test.py diff --git a/.claude/settings.json b/.claude/settings.json index 43a88656a..4e066c25a 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,4 +1,7 @@ { + "env": { + "CLAUDE_CODE_MAX_OUTPUT_TOKENS": "64000" + }, "hooks": { "SessionStart": [ { diff --git a/apps/minds/imbue/minds/cli/update_test.py b/apps/minds/imbue/minds/cli/update_test.py deleted file mode 100644 index cfb818806..000000000 --- a/apps/minds/imbue/minds/cli/update_test.py +++ /dev/null @@ -1,39 +0,0 @@ -import json - -from imbue.minds.config.data_types import parse_agents_from_mng_output - - -def test_parse_agents_from_mng_output_extracts_records() -> None: - """Verify parse_agents_from_mng_output extracts agent records from JSON.""" - json_str = json.dumps( - { - "agents": [ - {"id": "agent-abc123", "name": "selene", "work_dir": "/tmp/minds/selene"}, - ] - } - ) - agents = parse_agents_from_mng_output(json_str) - assert len(agents) == 1 - assert agents[0]["id"] == "agent-abc123" - assert agents[0]["name"] == "selene" - - -def test_parse_agents_from_mng_output_handles_empty() -> None: - """Verify parse_agents_from_mng_output returns empty list for no agents.""" - json_str = json.dumps({"agents": []}) - agents = parse_agents_from_mng_output(json_str) - assert agents == [] - - -def test_parse_agents_from_mng_output_handles_non_json() -> None: - """Verify parse_agents_from_mng_output handles non-JSON output gracefully.""" - agents = parse_agents_from_mng_output("not json at all") - assert agents == [] - - -def test_parse_agents_from_mng_output_handles_mixed_output() -> None: - """Verify parse_agents_from_mng_output handles SSH errors mixed with JSON.""" - output = "WARNING: some SSH error\n" + json.dumps({"agents": [{"id": "agent-xyz", "name": "test"}]}) - agents = parse_agents_from_mng_output(output) - assert len(agents) == 1 - assert agents[0]["id"] == "agent-xyz" diff --git a/apps/minds/imbue/minds/config/data_types_test.py b/apps/minds/imbue/minds/config/data_types_test.py index d6f34fda9..cb4c72997 100644 --- a/apps/minds/imbue/minds/config/data_types_test.py +++ b/apps/minds/imbue/minds/config/data_types_test.py @@ -1,7 +1,9 @@ +import json from pathlib import Path from imbue.minds.config.data_types import MindPaths from imbue.minds.config.data_types import get_default_data_dir +from imbue.minds.config.data_types import parse_agents_from_mng_output from imbue.mng.primitives import AgentId @@ -24,3 +26,42 @@ def test_get_default_data_dir_returns_home_minds() -> None: result = get_default_data_dir() assert result.name == ".minds" assert result.parent == Path.home() + + +# -- parse_agents_from_mng_output tests -- + + +def test_parse_agents_from_mng_output_extracts_records() -> None: + """Verify parse_agents_from_mng_output extracts agent records from JSON.""" + json_str = json.dumps( + { + "agents": [ + {"id": "agent-abc123", "name": "selene", "work_dir": "/tmp/minds/selene"}, + ] + } + ) + agents = parse_agents_from_mng_output(json_str) + assert len(agents) == 1 + assert agents[0]["id"] == "agent-abc123" + assert agents[0]["name"] == "selene" + + +def test_parse_agents_from_mng_output_handles_empty() -> None: + """Verify parse_agents_from_mng_output returns empty list for no agents.""" + json_str = json.dumps({"agents": []}) + agents = parse_agents_from_mng_output(json_str) + assert agents == [] + + +def test_parse_agents_from_mng_output_handles_non_json() -> None: + """Verify parse_agents_from_mng_output handles non-JSON output gracefully.""" + agents = parse_agents_from_mng_output("not json at all") + assert agents == [] + + +def test_parse_agents_from_mng_output_handles_mixed_output() -> None: + """Verify parse_agents_from_mng_output handles SSH errors mixed with JSON.""" + output = "WARNING: some SSH error\n" + json.dumps({"agents": [{"id": "agent-xyz", "name": "test"}]}) + agents = parse_agents_from_mng_output(output) + assert len(agents) == 1 + assert agents[0]["id"] == "agent-xyz" From 5bf8dc6a755add70ada51ffc6e6484a11bacc25c Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:27:57 -0700 Subject: [PATCH 7/8] Fix reviewer round 4: error types, domain types, add unit tests - Add GitOperationError base class with ParentTrackingError subclass; VendorError now inherits from GitOperationError - Add error_class parameter to run_git so callers can specify the appropriate error type (defaults to VendorError for compatibility) - parent_tracking.py now raises ParentTrackingError instead of VendorError - Use AgentName and GitUrl domain types in setup_mind_branch_and_parent and checkout_mind_branch signatures - Extract parse_mind_agent_record as a testable function - Add unit tests for MindAgentRecord and parse_mind_agent_record - Restore CLAUDE_CODE_MAX_OUTPUT_TOKENS env var in .claude/settings.json Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/minds/imbue/minds/cli/update.py | 10 ++++- apps/minds/imbue/minds/cli/update_test.py | 40 +++++++++++++++++++ apps/minds/imbue/minds/errors.py | 14 ++++++- .../minds/forwarding_server/agent_creator.py | 2 +- .../forwarding_server/parent_tracking.py | 33 +++++++++++---- .../forwarding_server/parent_tracking_test.py | 13 +++--- .../minds/forwarding_server/vendor_mng.py | 8 +++- 7 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 apps/minds/imbue/minds/cli/update_test.py diff --git a/apps/minds/imbue/minds/cli/update.py b/apps/minds/imbue/minds/cli/update.py index 0637a534e..b89dee5bf 100644 --- a/apps/minds/imbue/minds/cli/update.py +++ b/apps/minds/imbue/minds/cli/update.py @@ -72,7 +72,15 @@ def find_mind_agent(agent_name: str) -> MindAgentRecord: if not agents: raise MindError("No mind found with name '{}'".format(agent_name)) - raw = agents[0] + return parse_mind_agent_record(agents[0], agent_name) + + +def parse_mind_agent_record(raw: dict[str, object], agent_name: str) -> MindAgentRecord: + """Parse a raw agent dict from ``mng list`` JSON into a MindAgentRecord. + + Validates that the required ``id`` and ``work_dir`` fields are present. + Raises MindError if either field is missing. + """ raw_id = raw.get("id") raw_work_dir = raw.get("work_dir") if raw_id is None or raw_work_dir is None: diff --git a/apps/minds/imbue/minds/cli/update_test.py b/apps/minds/imbue/minds/cli/update_test.py new file mode 100644 index 000000000..fa9a6bbf0 --- /dev/null +++ b/apps/minds/imbue/minds/cli/update_test.py @@ -0,0 +1,40 @@ +from pathlib import Path + +import pytest + +from imbue.minds.cli.update import MindAgentRecord +from imbue.minds.cli.update import parse_mind_agent_record +from imbue.minds.errors import MindError +from imbue.mng.primitives import AgentId + + +def test_mind_agent_record_stores_fields() -> None: + """Verify MindAgentRecord stores agent_id and work_dir.""" + agent_id = AgentId() + record = MindAgentRecord(agent_id=agent_id, work_dir=Path("/tmp/test")) + assert record.agent_id == agent_id + assert record.work_dir == Path("/tmp/test") + + +def test_parse_mind_agent_record_extracts_fields() -> None: + """Verify parse_mind_agent_record extracts id and work_dir from raw dict.""" + valid_id = "agent-" + "a" * 32 + raw = {"id": valid_id, "name": "selene", "work_dir": "/tmp/minds/selene"} + record = parse_mind_agent_record(raw, "selene") + assert str(record.agent_id) == valid_id + assert record.work_dir == Path("/tmp/minds/selene") + + +def test_parse_mind_agent_record_raises_on_missing_id() -> None: + """Verify parse_mind_agent_record raises MindError when id is missing.""" + raw: dict[str, object] = {"name": "selene", "work_dir": "/tmp/minds/selene"} + with pytest.raises(MindError, match="missing required fields"): + parse_mind_agent_record(raw, "selene") + + +def test_parse_mind_agent_record_raises_on_missing_work_dir() -> None: + """Verify parse_mind_agent_record raises MindError when work_dir is missing.""" + valid_id = "agent-" + "a" * 32 + raw: dict[str, object] = {"id": valid_id, "name": "selene"} + with pytest.raises(MindError, match="missing required fields"): + parse_mind_agent_record(raw, "selene") diff --git a/apps/minds/imbue/minds/errors.py b/apps/minds/imbue/minds/errors.py index b164e0096..24403ab21 100644 --- a/apps/minds/imbue/minds/errors.py +++ b/apps/minds/imbue/minds/errors.py @@ -30,12 +30,24 @@ class MngCommandError(MindError): ... -class VendorError(MindError): +class GitOperationError(MindError): + """Raised when a git operation fails during mind management.""" + + ... + + +class VendorError(GitOperationError): """Raised when vendoring a repo into a mind fails.""" ... +class ParentTrackingError(GitOperationError): + """Raised when a parent tracking git operation fails.""" + + ... + + class DirtyRepoError(VendorError): """Raised when a local vendor repo has uncommitted changes or untracked files.""" diff --git a/apps/minds/imbue/minds/forwarding_server/agent_creator.py b/apps/minds/imbue/minds/forwarding_server/agent_creator.py index 00cb8770e..37606951a 100644 --- a/apps/minds/imbue/minds/forwarding_server/agent_creator.py +++ b/apps/minds/imbue/minds/forwarding_server/agent_creator.py @@ -276,7 +276,7 @@ def _create_agent_background( clone_git_repo(GitUrl(git_url), mind_dir, on_output=emit_log) log_queue.put("[minds] Setting up branch and parent tracking...") - setup_mind_branch_and_parent(mind_dir, agent_name, git_url, on_output=emit_log) + setup_mind_branch_and_parent(mind_dir, AgentName(agent_name), GitUrl(git_url), on_output=emit_log) settings = load_creation_settings(mind_dir) diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking.py index cf3b15b5c..454c12926 100644 --- a/apps/minds/imbue/minds/forwarding_server/parent_tracking.py +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking.py @@ -23,8 +23,10 @@ from pydantic import Field from imbue.imbue_common.frozen_model import FrozenModel +from imbue.minds.errors import ParentTrackingError from imbue.minds.forwarding_server.vendor_mng import ensure_git_identity from imbue.minds.forwarding_server.vendor_mng import run_git +from imbue.minds.primitives import AgentName from imbue.minds.primitives import GitBranch from imbue.minds.primitives import GitCommitHash from imbue.minds.primitives import GitUrl @@ -33,6 +35,8 @@ MIND_BRANCH_PREFIX: Final[str] = "minds/" +_ERR = ParentTrackingError + class ParentInfo(FrozenModel): """Lineage information for a mind's parent repository. @@ -56,6 +60,7 @@ def get_current_branch(repo_dir: Path) -> GitBranch: ["rev-parse", "--abbrev-ref", "HEAD"], cwd=repo_dir, error_message="Failed to get current branch of {}".format(repo_dir), + error_class=_ERR, ).strip() ) @@ -67,18 +72,19 @@ def get_current_commit_hash(repo_dir: Path) -> GitCommitHash: ["rev-parse", "HEAD"], cwd=repo_dir, error_message="Failed to get current commit hash of {}".format(repo_dir), + error_class=_ERR, ).strip() ) def checkout_mind_branch( repo_dir: Path, - mind_name: str, + mind_name: AgentName, on_output: Callable[[str, bool], None] | None = None, ) -> None: """Create and switch to a new branch named ``minds/``. - Raises VendorError if the branch already exists or the checkout fails. + Raises ParentTrackingError if the branch already exists or the checkout fails. """ branch_name = "{}{}".format(MIND_BRANCH_PREFIX, mind_name) logger.debug("Checking out new branch: {}", branch_name) @@ -87,6 +93,7 @@ def checkout_mind_branch( cwd=repo_dir, on_output=on_output, error_message="Failed to create branch {}".format(branch_name), + error_class=_ERR, ) @@ -114,25 +121,28 @@ def write_parent_info( cwd=repo_dir, on_output=on_output, error_message="Failed to write parent.url to {}".format(parent_file), + error_class=_ERR, ) run_git( ["config", "--file", parent_file, "parent.branch", str(parent_info.branch)], cwd=repo_dir, on_output=on_output, error_message="Failed to write parent.branch to {}".format(parent_file), + error_class=_ERR, ) run_git( ["config", "--file", parent_file, "parent.hash", str(parent_info.hash)], cwd=repo_dir, on_output=on_output, error_message="Failed to write parent.hash to {}".format(parent_file), + error_class=_ERR, ) def read_parent_info(repo_dir: Path) -> ParentInfo: """Read parent lineage information from the ``.parent`` file. - Raises VendorError if the file does not exist or cannot be read. + Raises ParentTrackingError if the file does not exist or cannot be read. """ parent_file = str(repo_dir / PARENT_FILE_NAME) @@ -140,16 +150,19 @@ def read_parent_info(repo_dir: Path) -> ParentInfo: ["config", "--file", parent_file, "parent.url"], cwd=repo_dir, error_message="Failed to read parent.url from {}".format(parent_file), + error_class=_ERR, ).strip() branch = run_git( ["config", "--file", parent_file, "parent.branch"], cwd=repo_dir, error_message="Failed to read parent.branch from {}".format(parent_file), + error_class=_ERR, ).strip() hash_value = run_git( ["config", "--file", parent_file, "parent.hash"], cwd=repo_dir, error_message="Failed to read parent.hash from {}".format(parent_file), + error_class=_ERR, ).strip() return ParentInfo(url=GitUrl(url), branch=GitBranch(branch), hash=GitCommitHash(hash_value)) @@ -171,6 +184,7 @@ def commit_parent_file( cwd=repo_dir, on_output=on_output, error_message="Failed to stage {}".format(PARENT_FILE_NAME), + error_class=_ERR, ) # Check if there are staged changes before committing @@ -178,6 +192,7 @@ def commit_parent_file( ["diff", "--cached", "--name-only"], cwd=repo_dir, error_message="Failed to check staged changes", + error_class=_ERR, ).strip() if not diff_output: @@ -189,13 +204,14 @@ def commit_parent_file( cwd=repo_dir, on_output=on_output, error_message="Failed to commit {}".format(PARENT_FILE_NAME), + error_class=_ERR, ) def setup_mind_branch_and_parent( repo_dir: Path, - mind_name: str, - git_url: str, + mind_name: AgentName, + git_url: GitUrl, on_output: Callable[[str, bool], None] | None = None, ) -> None: """Set up a mind's branch and parent tracking after cloning. @@ -213,7 +229,7 @@ def setup_mind_branch_and_parent( checkout_mind_branch(repo_dir, mind_name, on_output) - parent_info = ParentInfo(url=GitUrl(git_url), branch=parent_branch, hash=parent_hash) + parent_info = ParentInfo(url=git_url, branch=parent_branch, hash=parent_hash) write_parent_info(repo_dir, parent_info, on_output) commit_parent_file(repo_dir, on_output) @@ -231,7 +247,7 @@ def fetch_and_merge_parent( Returns the new parent commit hash (the fetched HEAD). - Raises VendorError if the fetch or merge fails (e.g. due to conflicts). + Raises ParentTrackingError if the fetch or merge fails (e.g. due to conflicts). """ ensure_git_identity(repo_dir) @@ -241,6 +257,7 @@ def fetch_and_merge_parent( cwd=repo_dir, on_output=on_output, error_message="Failed to fetch from {} branch {}".format(parent_info.url, parent_info.branch), + error_class=_ERR, ) new_hash = GitCommitHash( @@ -248,6 +265,7 @@ def fetch_and_merge_parent( ["rev-parse", "FETCH_HEAD"], cwd=repo_dir, error_message="Failed to resolve FETCH_HEAD", + error_class=_ERR, ).strip() ) @@ -257,6 +275,7 @@ def fetch_and_merge_parent( cwd=repo_dir, on_output=on_output, error_message="Failed to merge parent changes (there may be conflicts to resolve manually)", + error_class=_ERR, ) updated_info = ParentInfo(url=parent_info.url, branch=parent_info.branch, hash=new_hash) diff --git a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py index 1f0861708..7d483ae46 100644 --- a/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py +++ b/apps/minds/imbue/minds/forwarding_server/parent_tracking_test.py @@ -2,7 +2,7 @@ import pytest -from imbue.minds.errors import VendorError +from imbue.minds.errors import ParentTrackingError from imbue.minds.forwarding_server.parent_tracking import MIND_BRANCH_PREFIX from imbue.minds.forwarding_server.parent_tracking import PARENT_FILE_NAME from imbue.minds.forwarding_server.parent_tracking import ParentInfo @@ -15,6 +15,7 @@ from imbue.minds.forwarding_server.parent_tracking import setup_mind_branch_and_parent from imbue.minds.forwarding_server.parent_tracking import write_parent_info from imbue.minds.forwarding_server.vendor_mng import run_git +from imbue.minds.primitives import AgentName from imbue.minds.primitives import GitBranch from imbue.minds.primitives import GitCommitHash from imbue.minds.primitives import GitUrl @@ -38,7 +39,7 @@ def test_get_current_commit_hash_returns_full_hash(tmp_path: Path) -> None: def test_checkout_mind_branch_creates_new_branch(tmp_path: Path) -> None: repo = make_git_repo(tmp_path) - checkout_mind_branch(repo, "selene") + checkout_mind_branch(repo, AgentName("selene")) branch = get_current_branch(repo) assert branch == "{}selene".format(MIND_BRANCH_PREFIX) @@ -57,7 +58,7 @@ def test_write_and_read_parent_info_roundtrips(tmp_path: Path) -> None: def test_read_parent_info_raises_when_file_missing(tmp_path: Path) -> None: repo = make_git_repo(tmp_path) - with pytest.raises(VendorError, match="Failed to read parent.url"): + with pytest.raises(ParentTrackingError, match="Failed to read parent.url"): read_parent_info(repo) @@ -84,7 +85,7 @@ def test_setup_mind_branch_and_parent_full_flow(tmp_path: Path) -> None: run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") - setup_mind_branch_and_parent(clone_dir, "selene", str(source)) + setup_mind_branch_and_parent(clone_dir, AgentName("selene"), GitUrl(str(source))) # Verify branch branch = get_current_branch(clone_dir) @@ -107,7 +108,7 @@ def test_fetch_and_merge_parent_merges_changes(tmp_path: Path) -> None: run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") - setup_mind_branch_and_parent(clone_dir, "selene", str(source)) + setup_mind_branch_and_parent(clone_dir, AgentName("selene"), GitUrl(str(source))) # Make a change in the source repo (source / "new_file.txt").write_text("new content") @@ -133,7 +134,7 @@ def test_fetch_and_merge_parent_noop_when_up_to_date(tmp_path: Path) -> None: clone_dir = tmp_path / "clone" run_git(["clone", str(source), str(clone_dir)], cwd=tmp_path, error_message="clone failed") - setup_mind_branch_and_parent(clone_dir, "selene", str(source)) + setup_mind_branch_and_parent(clone_dir, AgentName("selene"), GitUrl(str(source))) parent_info = read_parent_info(clone_dir) new_hash = fetch_and_merge_parent(clone_dir, parent_info) diff --git a/apps/minds/imbue/minds/forwarding_server/vendor_mng.py b/apps/minds/imbue/minds/forwarding_server/vendor_mng.py index 0ded7e23e..63c6cdc5c 100644 --- a/apps/minds/imbue/minds/forwarding_server/vendor_mng.py +++ b/apps/minds/imbue/minds/forwarding_server/vendor_mng.py @@ -20,6 +20,7 @@ from imbue.concurrency_group.concurrency_group import ConcurrencyGroup from imbue.imbue_common.primitives import NonEmptyStr from imbue.minds.errors import DirtyRepoError +from imbue.minds.errors import GitOperationError from imbue.minds.errors import VendorError from imbue.mng_claude_mind.data_types import VendorRepoConfig @@ -274,10 +275,13 @@ def run_git( cwd: Path, on_output: Callable[[str, bool], None] | None = None, error_message: str = "git command failed", + error_class: type[GitOperationError] = VendorError, ) -> str: """Run a git command and return stdout. - Raises VendorError if the command exits with a non-zero status. + Raises the specified error class (default: VendorError) if the command + exits with a non-zero status. Callers can pass a different error class + for semantically appropriate error types. """ cg = ConcurrencyGroup(name="vendor-git") with cg: @@ -288,7 +292,7 @@ def run_git( on_output=on_output, ) if result.returncode != 0: - raise VendorError( + raise error_class( "{} (exit code {}):\n{}".format( error_message, result.returncode, From cf6fd48a9a41e16b8404e9c3daef7701b3546c97 Mon Sep 17 00:00:00 2001 From: Josh Albrecht Date: Tue, 17 Mar 2026 10:31:17 -0700 Subject: [PATCH 8/8] Catch GitOperationError in agent creation to handle ParentTrackingError ParentTrackingError inherits from GitOperationError (not VendorError), so the except block needs to catch GitOperationError to handle failures from both vendor operations and parent tracking operations. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/minds/imbue/minds/forwarding_server/agent_creator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/minds/imbue/minds/forwarding_server/agent_creator.py b/apps/minds/imbue/minds/forwarding_server/agent_creator.py index 37606951a..f01cfa023 100644 --- a/apps/minds/imbue/minds/forwarding_server/agent_creator.py +++ b/apps/minds/imbue/minds/forwarding_server/agent_creator.py @@ -30,8 +30,8 @@ from imbue.minds.config.data_types import MNG_BINARY from imbue.minds.config.data_types import MindPaths from imbue.minds.errors import GitCloneError +from imbue.minds.errors import GitOperationError from imbue.minds.errors import MngCommandError -from imbue.minds.errors import VendorError from imbue.minds.forwarding_server.parent_tracking import setup_mind_branch_and_parent from imbue.minds.forwarding_server.vendor_mng import default_vendor_configs from imbue.minds.forwarding_server.vendor_mng import find_mng_repo_root @@ -308,7 +308,7 @@ def _create_agent_background( self._statuses[aid] = AgentCreationStatus.DONE self._redirect_urls[aid] = "/agents/{}/".format(agent_id) - except (GitCloneError, MngCommandError, VendorError, ValueError, OSError) as e: + except (GitCloneError, MngCommandError, GitOperationError, ValueError, OSError) as e: logger.error("Failed to create agent {}: {}", agent_id, e) log_queue.put("[minds] ERROR: {}".format(e)) with self._lock: