diff --git a/ms_agent/agent_hub/_commands.py b/ms_agent/agent_hub/_commands.py index ca90da090..1ab9e3761 100644 --- a/ms_agent/agent_hub/_commands.py +++ b/ms_agent/agent_hub/_commands.py @@ -54,8 +54,7 @@ def _print_openhuman_next_steps(root) -> None: 'system_prompt.inline（粘贴 SOUL.md 内容）/ model.exact="inherit"；id 仅限字母、数字、"_"、"-"。\n' ' 2) UI「设置 → 智能体 → 新建智能体」，id / model / 工具白名单与 TOML 保持一致。\n' ' 3) UI「设置 → 智能体配置 → 新建配置」，base agent id 填同一个 id。\n' - ' 4) 新建一个对话即可使用。' - ) + ' 4) 新建一个对话即可使用。') logger.info( 'Conversion/download finished, but the agent is NOT usable yet ' '(if you have not registered it). OpenHuman discovers agents only via the ' @@ -68,8 +67,7 @@ def _print_openhuman_next_steps(root) -> None: 'id may contain only letters, digits, "_", "-".\n' ' 2) UI "Settings → Agents → New Agent": keep id / model / tool allowlist consistent with the TOML.\n' ' 3) UI "Settings → Agent Config → New Config": set base agent id to the same id.\n' - ' 4) Start a new conversation.' - ) + ' 4) Start a new conversation.') def _fail(message: str) -> int: @@ -289,10 +287,10 @@ def convert_resources( source_defaults=get_defaults(source_fw), target_defaults=get_defaults(target_fw), fill_missing_defaults=fill_missing_defaults, - overflow_target=(_file_per_agent_identity_path(dst_spec) - if dst_spec is not None else None), - identity_source=(_file_per_agent_identity_path(src_spec) - if src_spec is not None else None), + overflow_target=(_file_per_agent_identity_path( + dst_spec, for_target=True) if dst_spec is not None else None), + identity_source=(_file_per_agent_identity_path( + src_spec, for_target=False) if src_spec is not None else None), existing_skills=(_existing_skill_names(existing_files) if existing_files else None), ) @@ -832,8 +830,8 @@ def cmd_download( from ._sync import backup_local if spec.collect(): backup_label = ( - target_fw if local_name == ALL_AGENT_NAME else - f'{target_fw}_{local_name}') + target_fw + if local_name == ALL_AGENT_NAME else f'{target_fw}_{local_name}') display.meta('backup', backup_local(spec, backup_label)) written = spec.apply(filtered) @@ -850,18 +848,27 @@ def cmd_download( return 0 -def _file_per_agent_identity_path(dst_spec: WorkspaceSpec) -> str | None: - """Resolve the per-agent identity file for a file-per-agent target. +def _file_per_agent_identity_path(spec: WorkspaceSpec, *, + for_target: bool) -> str | None: + """Resolve the per-agent identity file of a file-per-agent layout. File-per-agent frameworks (e.g. qoder) declare a ``{name}`` placeholder - pattern such as ``agents/{name}.md``. Format it with the destination - agent name so converted persona content can be routed into that file. - Returns ``None`` when the layout has no single ``{name}`` file pattern. + pattern such as ``agents/{name}.md``. Format it with the agent name so + persona content can be routed there. Returns ``None`` when the layout + has no single ``{name}`` file pattern. + + ``for_target=True`` resolves the overflow target (where converted persona + lands); qoder opts out there: imported persona folds into the shared + ``AGENTS.md`` instead of spawning ``agents/.md``. + ``for_target=False`` resolves the identity source (flag this layout's own + persona file so it folds into the target persona instead of being + dropped); qoder must stay in there -- skipping it on the source side + loses the persona on every qoder->X conversion. """ - if dst_spec.product_name == 'qoder': + if for_target and spec.product_name == 'qoder': return None - name = dst_spec.agent_name or DEFAULT_AGENT_NAME - for pattern in dst_spec.patterns: + name = spec.agent_name or DEFAULT_AGENT_NAME + for pattern in spec.patterns: # Only single-file placeholders (no wildcard) identify the persona file; # skip glob patterns like ``skills/{name}/*`` if any exist. if '{name}' in pattern and '*' not in pattern: @@ -973,15 +980,18 @@ def convert_workspace( default_paths = set() skipped_skills = [] else: - # File-per-agent targets (e.g. qoder ``agents/{name}.md``) keep - # per-agent identity in a dedicated sub-agent file; route overflow - # (persona content with no shared mapping) there instead of the - # shared catch-all so it does not pollute other sub-agents. - # Symmetrically, a file-per-agent SOURCE's persona file is flagged so - # it folds into the target's persona file instead of being dropped. + # File-per-agent targets declare a per-agent identity file + # (``agents/{name}.md``); overflow (persona content with no shared + # mapping) is routed there instead of the shared catch-all so it does + # not pollute other sub-agents. qoder opts out on the target side: + # its imported persona folds into the shared AGENTS.md instead (see + # _file_per_agent_identity_path). Symmetrically, a file-per-agent + # SOURCE's persona file is flagged so it folds into the target's + # persona file instead of being dropped. overflow_target = None if any('{name}' in p for p in dst_spec.patterns): - overflow_target = _file_per_agent_identity_path(dst_spec) + overflow_target = _file_per_agent_identity_path( + dst_spec, for_target=True) result = merge_resources( incoming=resources, source_product=source_fw, @@ -989,7 +999,8 @@ def convert_workspace( source_defaults=get_defaults(source_fw), target_defaults=get_defaults(target_fw), overflow_target=overflow_target, - identity_source=_file_per_agent_identity_path(src_spec), + identity_source=_file_per_agent_identity_path( + src_spec, for_target=False), fill_missing_defaults=False, existing_skills=_existing_skill_names(existing_paths), ) @@ -1116,7 +1127,14 @@ def cmd_convert( if err: return _fail(err) - src_name = from_name or DEFAULT_AGENT_NAME + if from_name: + src_name = from_name + else: + # An omitted --from-name asks the framework which agent it should + # convert: frameworks with an "active" sub-agent notion (openhuman's + # activeProfileId) return it, everything else returns ``default``. + src_name = build_spec(source_fw, DEFAULT_AGENT_NAME, + local_dir).resolve_default_agent_name() dst_name = target_name or src_name src_spec = build_spec(source_fw, src_name, local_dir) dst_spec = build_spec(target_fw, dst_name, out_dir) diff --git a/ms_agent/agent_hub/_merge.py b/ms_agent/agent_hub/_merge.py index b725ef160..538c87d9a 100644 --- a/ms_agent/agent_hub/_merge.py +++ b/ms_agent/agent_hub/_merge.py @@ -485,13 +485,21 @@ def merge( # every file verbatim by design). PRODUCT_PRIVATE_FILES = { 'hermes': frozenset(['config.yaml', 'hooks/*']), - 'ms-agent': - frozenset(['settings.json', 'skills.json', - 'mcp.json']), + 'ms-agent': frozenset(['settings.json', 'skills.json', 'mcp.json']), 'qwenpaw': frozenset(['agent.json', 'skill.json']), 'openhuman': frozenset(['config.toml']), } +# openhuman records HOW a skill was installed in a per-skill sidecar: +# ``_meta.json`` (marketplace entry: owner/slug/publishedAt) and +# ``metadata.json`` (github source: repo/path/downloaded_at). Both are +# openhuman-private provenance with no meaning to other frameworks, so a +# cross-framework convert drops them while same-framework sync keeps them +# (BUG-0828). +_SKILL_PROVENANCE_FILES = { + 'openhuman': frozenset(['_meta.json', 'metadata.json']), +} + def _is_private_file(product: str, path: str) -> bool: """Whether *path* is a framework-private (non-portable) file of *product*. @@ -775,6 +783,19 @@ def merge_resources( if skill_path.startswith('skills/'): parts = skill_path.split('/') skill_name = parts[1] if len(parts) > 1 else '' + # Framework-private per-skill provenance sidecars never travel + # across frameworks (BUG-0828). + if (is_cross_product and len(parts) == 3 + and parts[2] in _SKILL_PROVENANCE_FILES.get( + source_product, ())): + result.actions.append( + MergeAction( + path=path, + action='skip', + detail=(f'{path} is {source_product}-private skill ' + f'provenance, dropped'), + )) + continue if skill_name in existing_skill_set: result.actions.append( MergeAction( diff --git a/ms_agent/agent_hub/_workspace.py b/ms_agent/agent_hub/_workspace.py index 1ee1580a9..d1ee571f4 100644 --- a/ms_agent/agent_hub/_workspace.py +++ b/ms_agent/agent_hub/_workspace.py @@ -479,6 +479,18 @@ def list_agents(self) -> list[str]: """ return [DEFAULT_AGENT_NAME] + def resolve_default_agent_name(self) -> str: + """Which agent an omitted ``--name`` should operate on. + + Default: the ``default`` agent. Frameworks that keep a notion of an + *active* sub-agent (e.g. openhuman's ``activeProfileId``) override + this so a name-less convert picks the persona the user is actually + working with instead of the bare default. Must never raise: an + absent / unreadable active-agent marker falls back to + ``DEFAULT_AGENT_NAME``. + """ + return DEFAULT_AGENT_NAME + def _list_agents_from_dir(self, agents_dir: Path) -> list[str]: """List agents from a directory, prepending DEFAULT if not present.""" agents = _list_agent_files(agents_dir) diff --git a/ms_agent/agent_hub/frameworks/_bundled_skills.py b/ms_agent/agent_hub/frameworks/_bundled_skills.py index ae66abb27..4159e1bdd 100644 --- a/ms_agent/agent_hub/frameworks/_bundled_skills.py +++ b/ms_agent/agent_hub/frameworks/_bundled_skills.py @@ -24,14 +24,16 @@ class BundledSkillFilterMixin: A ``skills//`` is treated as framework-provided when its ``SKILL.md`` frontmatter carries any of: a ``name`` listed in the sibling ``.bundled_manifest`` (Hermes's content library); a ``builtin_skill_version`` - field; or a ``metadata`` block whose nested product entry carries install - hints (``emoji``/``requires``/``install``). User skills carry none of these - and are the only ones kept. A bare ``license`` field or a bare - ``metadata.`` key is deliberately NOT a marker: both appear on - user-authored skills (open-source license, custom per-product parameters) - and treating them as "bundled" silently dropped those skills - (BUG-021/BUG-022). Only the ``skills/`` tree is filtered (Hermes's - ``optional-skills/`` is left untouched). + field; a ``metadata`` block whose nested product entry carries install + hints (``emoji``/``requires``/``install``); or a ``metadata.hermes`` + catalog block carrying ``tags`` (Hermes app-native skills seeded outside + the manifest, BUG-0828). User skills carry none of these and are the only + ones kept. A bare ``license`` field or a bare ``metadata.`` key is + deliberately NOT a marker: both appear on user-authored skills + (open-source license, custom per-product parameters) and treating them as + "bundled" silently dropped those skills (BUG-021/BUG-022). Only the + ``skills/`` tree is filtered (Hermes's ``optional-skills/`` is left + untouched). """ def _walk_matched(self): @@ -113,6 +115,17 @@ def _is_framework_skill(self, skill_md: Path, bundled: frozenset) -> bool: for v in md.values(): if isinstance(v, dict) and (v.keys() & _BUNDLED_SKILL_KEYS): return True + # Hermes app-native skills (desktop plugins, themes, the + # ``apple/`` / ``media/`` / ``mlops/`` category libraries) are + # seeded by the app OUTSIDE the ``.bundled_manifest`` sync, so + # neither the manifest nor the install-hint keys above catch + # them. They carry a ``metadata.hermes`` catalog block; ``tags`` + # is the shape marker. A bare ``hermes`` key or a + # ``metadata.hermes.config`` settings block is a legitimate user + # skill and must NOT be marked (BUG-022 precedent, BUG-0828). + hermes_md = md.get('hermes') + if isinstance(hermes_md, dict) and 'tags' in hermes_md: + return True return False def _user_skill_dirs(self, skills_rel: str) -> set: diff --git a/ms_agent/agent_hub/frameworks/openhuman.py b/ms_agent/agent_hub/frameworks/openhuman.py index d7cdfdd15..68b87097a 100644 --- a/ms_agent/agent_hub/frameworks/openhuman.py +++ b/ms_agent/agent_hub/frameworks/openhuman.py @@ -2,12 +2,17 @@ """OpenHuman workspace specification (single-agent install).""" from __future__ import annotations +import copy +import json import re from pathlib import Path +from ms_agent.utils.logger import get_logger from .._workspace import (DEFAULT_AGENT_NAME, WorkspaceSpec, is_secret_key, register_framework) +logger = get_logger() + class OpenhumanWorkspace(WorkspaceSpec): """Workspace spec for the OpenHuman agent framework (root-per-agent). @@ -49,9 +54,31 @@ class OpenhumanWorkspace(WorkspaceSpec): # Per-device user workspace: ``users//workspace``. The id segment # is machine-generated, so it is discovered by scanning rather than named. + _DATA_DIRNAME = '.openhuman' _USERS_DIRNAME = 'users' _WORKSPACE_DIRNAME = 'workspace' _PROFILES_DIRNAME = 'personalities' + _PROFILES_JSON_FILENAME = 'agent_profiles.json' + + # Liveness markers used to SCORE candidate user workspaces when several + # exist (app reinstalls / user-id scheme migrations leave stale siblings + # behind, and a sorted-first pick silently resolved to the stale one -- + # BUG-0828). ``agent_profiles.json`` is the profile registry the app only + # writes where a user actually runs; ``personalities/`` holds the Profile + # personas; ``SOUL.md`` is a weak signal (stale shells keep a copy too). + _LIVENESS_SCORES: tuple[tuple[str, int], ...] = ( + (_PROFILES_JSON_FILENAME, 4), + (_PROFILES_DIRNAME, 2), + ('SOUL.md', 1), + ) + + # Persona files that fall back to the workspace-level copy when a Profile + # does not carry its own -- the app's own lookup order (Profile file > + # workspace-level default). Deliberately limited to these four: + # ``config.toml`` is machine-local, and ``wiki/`` / ``skills/`` are large + # trees whose per-profile duplication would bloat upload/sync. + _WORKSPACE_FALLBACK_FILES = frozenset( + ['SOUL.md', 'IDENTITY.md', 'HEARTBEAT.md', 'MEMORY.md']) @property def product_name(self) -> str: @@ -61,31 +88,58 @@ def product_name(self) -> str: def default_root(self) -> Path: """Resolve the per-device user workspace under ``~/.openhuman``. - Returns ``/.openhuman/users//workspace`` for the single - installed user; when several user dirs exist the one with a - ``workspace/`` subdir wins (deterministic: first in sorted order). On a - fresh install with no ``users/`` tree yet, falls back to - ``~/.openhuman`` so ``status`` still reports a sensible path instead of - raising. + Returns ``/.openhuman/users//workspace`` for the + installed users; when several user dirs exist the LIVE one wins by + liveness score (see :meth:`_resolve_workspace`), with sorted order as + the deterministic tie-break. On a fresh install with no ``users/`` + tree yet, falls back to ``~/.openhuman`` so ``status`` still reports + a sensible path instead of raising. """ base = Path.home() / '.openhuman' return self._resolve_workspace(base) + @classmethod + def _workspace_score(cls, ws: Path) -> int: + """Liveness score of a candidate user workspace (higher = more likely + the one actually in use). See :data:`_LIVENESS_SCORES`.""" + score = 0 + for name, weight in cls._LIVENESS_SCORES: + if (ws / name).exists(): + score += weight + return score + @classmethod def _resolve_workspace(cls, base: Path) -> Path: """Find the ``users//workspace`` dir under *base*. Also accepts *base* already BEING a user workspace (it directly holds ``personalities/`` or the persona files), so an explicit ``local_dir`` - may point at either the ``.openhuman`` root or the workspace itself. + may point at either the ``.openhuman`` root or the workspace itself -- + or even one level ABOVE the data root (a backup dir that merely + CONTAINS ``.openhuman/``), which is descended into (BUG-0828). + + When several ``users//`` dirs exist, the LIVE workspace wins by + liveness score (``agent_profiles.json`` / ``personalities/`` / + ``SOUL.md``); ties and all-empty scores fall back to sorted order, so + a single-user or fresh install behaves exactly as before (BUG-0828: + a stale alphabetically-first user dir used to shadow the active one). """ users = base / cls._USERS_DIRNAME + if not users.is_dir(): + data_root = base / cls._DATA_DIRNAME + if (data_root / cls._USERS_DIRNAME).is_dir(): + users = data_root / cls._USERS_DIRNAME if users.is_dir(): - candidates = [d for d in sorted(users.iterdir()) if d.is_dir()] - for d in candidates: - ws = d / cls._WORKSPACE_DIRNAME - if ws.is_dir(): - return ws + candidates = sorted(d for d in users.iterdir() if d.is_dir()) + workspaces = [ + d / cls._WORKSPACE_DIRNAME for d in candidates + if (d / cls._WORKSPACE_DIRNAME).is_dir() + ] + if workspaces: + # ``max`` keeps the FIRST maximum in iteration order, and the + # list is sorted -- so ties deterministically fall back to the + # old sorted-first behavior. + return max(workspaces, key=cls._workspace_score) if candidates: return candidates[0] / cls._WORKSPACE_DIRNAME return base @@ -160,6 +214,79 @@ def list_agents(self) -> list[str]: agents = [DEFAULT_AGENT_NAME] + agents return agents + # ------------------------------------------------------------------ + # Active-profile auto-selection + # ------------------------------------------------------------------ + + def resolve_default_agent_name(self) -> str: + """Omitted ``--name`` selects the ACTIVE profile, not bare ``default``. + + The app keeps the currently selected persona in + ``agent_profiles.json`` (``activeProfileId``); converting without an + explicit name should migrate that persona, matching what the user + sees in the app. Strictly best-effort: a missing / malformed marker + or an id whose directory does not exist falls back to ``default`` + (the workspace-level persona) without raising. + """ + active = self._active_profile_id() + if active and active in self.list_agents(): + return active + return DEFAULT_AGENT_NAME + + def _active_profile_id(self) -> str | None: + """Read ``activeProfileId`` from ``agent_profiles.json`` (best effort). + + Returns ``None`` on any failure (file absent, unreadable, not JSON, + unexpected shape) -- callers treat that as "no active profile". + """ + path = self.root / self._PROFILES_JSON_FILENAME + try: + data = json.loads(path.read_text(encoding='utf-8')) + except (OSError, UnicodeDecodeError, ValueError): + return None + if not isinstance(data, dict): + return None + active = data.get('activeProfileId') + if not isinstance(active, str) or not active.strip(): + return None + return active.strip() + + # ------------------------------------------------------------------ + # Workspace-level persona fallback for Profile agents + # ------------------------------------------------------------------ + + def collect(self) -> dict[str, str]: + return self._with_workspace_fallbacks(super().collect(), text=True) + + def collect_bytes(self) -> dict[str, bytes]: + return self._with_workspace_fallbacks( + super().collect_bytes(), text=False) + + def _with_workspace_fallbacks(self, resources: dict, *, + text: bool) -> dict: + """Fill missing Profile files from the workspace-level copies. + + A Profile that lacks e.g. ``MEMORY.md`` runs with the workspace-level + one at runtime (app lookup order), so a converted agent must get it + too: missing files in :data:`_WORKSPACE_FALLBACK_FILES` are taken + from the workspace root when present there. Files the Profile already + has always win; all-mode is exempt (each Profile mirrors to its own + repo and workspace files would duplicate across every Profile). + """ + if self._is_all() or self.workspace_root == self.root: + return resources + workspace_spec = copy.copy(self) + workspace_spec.agent_name = DEFAULT_AGENT_NAME + for rel, f in workspace_spec._walk_matched(): + if rel not in self._WORKSPACE_FALLBACK_FILES or rel in resources: + continue + try: + resources[rel] = ( + f.read_text(encoding='utf-8') if text else f.read_bytes()) + except (OSError, UnicodeDecodeError) as e: + logger.warning('Skip workspace fallback %s: %s', f, e) + return resources + # ------------------------------------------------------------------ # config.toml secret sanitization (inbound + outbound) # ------------------------------------------------------------------ diff --git a/tests/agent_hub/test_cli.py b/tests/agent_hub/test_cli.py index 969399611..d2295ba67 100644 --- a/tests/agent_hub/test_cli.py +++ b/tests/agent_hub/test_cli.py @@ -1234,8 +1234,8 @@ def test_existing_skill_not_overwritten(self, mock_sync_cache): def test_download_convert_parity_with_local_convert(self): """Regression (BUG-020): ``download --target-framework`` must produce the SAME file set as a local ``convert`` -- same persona routing for - file-per-agent targets (agents/.md, no shared AGENTS.md - pollution) and no invented target default templates.""" + file-per-agent targets (folded into the shared AGENTS.md, no + per-agent file spawned) and no invented target default templates.""" from ms_agent.agent_hub._commands import cmd_convert src = Path(self.tmp.name) / "src" src.mkdir() @@ -1262,9 +1262,13 @@ def test_download_convert_parity_with_local_convert(self): cv_files = sorted( str(p.relative_to(cv)) for p in cv.rglob("*") if p.is_file()) self.assertEqual(dl_files, cv_files, f"target={target}") - # file-per-agent target: persona landed in its private file. - self.assertTrue((Path(self.tmp.name) / "dl_qoder" / "agents" - / "myagent.md").is_file()) + # file-per-agent target: persona folded into the shared AGENTS.md; + # no per-agent identity file spawned. + dl_agents_md = Path(self.tmp.name) / "dl_qoder" / "AGENTS.md" + self.assertTrue(dl_agents_md.is_file()) + self.assertIn("soul", dl_agents_md.read_text()) + self.assertFalse((Path(self.tmp.name) / "dl_qoder" / "agents" + / "myagent.md").is_file()) def test_download_without_login_fails(self): rc = cmd_download( diff --git a/tests/agent_hub/test_convert_targetname.py b/tests/agent_hub/test_convert_targetname.py index 2dc045acf..a1b7d3142 100644 --- a/tests/agent_hub/test_convert_targetname.py +++ b/tests/agent_hub/test_convert_targetname.py @@ -65,8 +65,9 @@ class TestConvertTargetNameLanding(unittest.TestCase): root-per-agent (openclaw/qwenpaw): target-name lands via directory prefix. single-agent (hermes): target-name has no path effect (by design). - file-per-agent (qoder): target-name lands in agents/{name}.md, - keeping the shared AGENTS.md clean. + file-per-agent (qoder): converted persona folds into the shared + AGENTS.md; --target-name has no path + effect. """ def setUp(self): @@ -170,12 +171,12 @@ def test_qwenpaw_to_hermes_targetname_lands_in_profiles(self): self.assertIn("SOUL.md", files) self.assertIn("Bot A creative AI.", files["SOUL.md"]) - def test_qwenpaw_to_qoder_targetname_lands_in_agents_file(self): - """file-per-agent target: --target-name lands in agents/{name}.md. + def test_qwenpaw_to_qoder_persona_folds_into_agents_md(self): + """file-per-agent target: converted persona folds into AGENTS.md. - The converted persona (SOUL/PROFILE) is routed to the per-agent file - agents/bot-a.md, while the shared AGENTS.md must NOT be polluted with - that identity content. + The imported persona (SOUL/PROFILE) folds into the shared AGENTS.md + instead of spawning agents/{name}.md; --target-name is still accepted + but no longer picks a landing file. """ out = self.base / "qoder_home" rc = cmd_convert( @@ -185,18 +186,16 @@ def test_qwenpaw_to_qoder_targetname_lands_in_agents_file(self): ) self.assertEqual(rc, 0) files = _read_all(out) - # Persona now lands in the dedicated per-agent file. - self.assertIn("agents/bot-a.md", files, - "file-per-agent target must route persona to agents/{name}.md") - self.assertIn("Bot A creative AI.", files["agents/bot-a.md"]) - self.assertIn("Bot A profile.", files["agents/bot-a.md"]) - # Shared AGENTS.md, if present, must not carry the imported persona. - if "AGENTS.md" in files: - self.assertNotIn("Bot A creative AI.", files["AGENTS.md"], - "shared AGENTS.md must stay free of per-agent identity") - - def test_qwenpaw_to_qoder_default_name_lands_in_agents_default(self): - """file-per-agent target without --target-name: persona -> agents/default.md.""" + # Persona folds into the shared AGENTS.md ... + self.assertIn("AGENTS.md", files, + "qoder target must fold persona into AGENTS.md") + self.assertIn("Bot A creative AI.", files["AGENTS.md"]) + self.assertIn("Bot A profile.", files["AGENTS.md"]) + # ... and no per-agent identity file is spawned. + self.assertNotIn("agents/bot-a.md", files) + + def test_qwenpaw_to_qoder_default_name_persona_folds_into_agents_md(self): + """file-per-agent target without --target-name: persona -> AGENTS.md.""" out = self.base / "qoder_default_home" # from_name=default -> source lives in the default sub-agent workspace. src_default = self.base / "src_default" @@ -211,9 +210,10 @@ def test_qwenpaw_to_qoder_default_name_lands_in_agents_default(self): ) self.assertEqual(rc, 0) files = _read_all(out) - self.assertIn("agents/default.md", files, - "default persona must land in agents/default.md") - self.assertIn("Bot A creative AI.", files["agents/default.md"]) + self.assertIn("AGENTS.md", files, + "default persona must fold into AGENTS.md") + self.assertIn("Bot A creative AI.", files["AGENTS.md"]) + self.assertNotIn("agents/default.md", files) # =========================================================================== @@ -617,7 +617,8 @@ def test_persona_survives_to_every_target(self): f"qoder->{target}: shared rules lost") def test_reverse_direction_not_regressed(self): - """hermes -> qoder still folds SOUL.md into agents/.md.""" + """hermes -> qoder still folds SOUL.md -- into the shared AGENTS.md, + not into agents/.md.""" with tempfile.TemporaryDirectory() as td: src = Path(td) / "src" src.mkdir() @@ -628,9 +629,10 @@ def test_reverse_direction_not_regressed(self): from_name="default", target_name="test-architect", local_dir=str(src), out_dir=str(out)) self.assertEqual(rc, 0) - persona = out / "agents" / "test-architect.md" - self.assertTrue(persona.is_file()) - self.assertIn("GOLD-HERMES-PERSONA", persona.read_text()) + shared = out / "AGENTS.md" + self.assertTrue(shared.is_file()) + self.assertIn("GOLD-HERMES-PERSONA", shared.read_text()) + self.assertFalse((out / "agents" / "test-architect.md").is_file()) class TestHermesOptionalSkillsOutbound(unittest.TestCase): @@ -804,20 +806,23 @@ def test_ms_agent_to_hermes_drops_config_and_skills_json(self): p.read_text() for p in out.rglob("*") if p.is_file()) self.assertIn("GOLD-PERSONA", all_text) - def test_openhuman_to_ms_agent_keeps_full_skill_tree(self): - """openhuman -> ms-agent: a skill is an atomic directory, so the whole - tree travels -- SKILL.md plus every sibling file, including per-skill - sidecars like ``_meta.json`` / ``metadata.json``. ms-agent only reads - SKILL.md, so extra files are harmless; the merger never guesses which - filenames are ``private`` and risks dropping a real dependency. - Regression for a report where skills vanished entirely. + def test_openhuman_to_ms_agent_keeps_skill_tree_drops_provenance(self): + """openhuman -> ms-agent: a skill is an atomic directory, so the tree + travels -- SKILL.md plus asset subdirs -- and skills must never vanish + (regression for a report where they were lost entirely). But + openhuman's per-skill PROVENANCE sidecars (``_meta.json`` marketplace + record, ``metadata.json`` github source) are framework-private and are + dropped on the cross-framework convert (BUG-0828); same-framework sync + still keeps them. """ with tempfile.TemporaryDirectory() as td: src = Path(td) / "src" - (src / "skills" / "weather").mkdir(parents=True) + (src / "skills" / "weather" / "references").mkdir(parents=True) (src / "skills" / "news-daily").mkdir(parents=True) (src / "SOUL.md").write_text("# Soul\nGOLD-PERSONA\n") (src / "skills" / "weather" / "SKILL.md").write_text("GOLD-WEATHER\n") + (src / "skills" / "weather" / "references" / "formats.md").write_text( + "GOLD-ASSET\n") (src / "skills" / "weather" / "_meta.json").write_text('{"k": 1}') (src / "skills" / "news-daily" / "SKILL.md").write_text("GOLD-NEWS\n") (src / "skills" / "news-daily" / "metadata.json").write_text('{"k": 2}') @@ -831,13 +836,15 @@ def test_openhuman_to_ms_agent_keeps_full_skill_tree(self): str(p.relative_to(out)) for p in out.rglob("*") if p.is_file() } - # the whole skill tree travels verbatim, sidecars included. + # SKILL.md and asset subdirs travel ... for expected in ( "skills/weather/SKILL.md", - "skills/weather/_meta.json", - "skills/news-daily/SKILL.md", - "skills/news-daily/metadata.json"): + "skills/weather/references/formats.md", + "skills/news-daily/SKILL.md"): self.assertIn(expected, rels) + # ... but the openhuman-private provenance sidecars are dropped. + self.assertNotIn("skills/weather/_meta.json", rels) + self.assertNotIn("skills/news-daily/metadata.json", rels) self.assertEqual( (out / "skills" / "weather" / "SKILL.md").read_text(), "GOLD-WEATHER\n") diff --git a/tests/agent_hub/test_workspace.py b/tests/agent_hub/test_workspace.py index ee29dad9a..9d817199a 100644 --- a/tests/agent_hub/test_workspace.py +++ b/tests/agent_hub/test_workspace.py @@ -77,6 +77,50 @@ def test_hermes_excludes_framework_skills_keeps_user_skills(self): self.assertNotIn("skills/docx/SKILL.md", collected) self.assertNotIn("skills/broken-bundled/SKILL.md", collected) + def test_hermes_metadata_hermes_tags_marks_app_native_skill(self): + """BUG-0828: Hermes app-native skills (desktop plugins, themes, the + ``apple/`` / ``media/`` category libraries) are seeded OUTSIDE the + ``.bundled_manifest`` sync and carry a ``metadata.hermes`` catalog + block -- ``tags`` is the shape marker. A ``metadata.hermes.config`` + settings block or a bare ``hermes`` key is a legitimate user skill + (BUG-022 precedent) and must stay. + """ + spec = build_spec("hermes", "default", str(self.root)) + base = spec.workspace_root + # App-native top-level skill: metadata.hermes.tags -> dropped. + (base / "skills" / "hermes-themes").mkdir(parents=True) + (base / "skills" / "hermes-themes" / "SKILL.md").write_text( + "---\nname: hermes-themes\nmetadata:\n hermes:\n" + " tags: [theme, skin]\n related_skills: []\n---\n# themes\n", + encoding="utf-8") + # App-native skill nested in a category dir, with an asset -> the + # whole tree is dropped. + cat = base / "skills" / "media" / "heartmula" + cat.mkdir(parents=True) + (cat / "SKILL.md").write_text( + "---\nname: heartmula\nmetadata:\n hermes:\n" + " tags: [music]\n---\n# heartmula\n", encoding="utf-8") + (cat / "references").mkdir() + (cat / "references" / "models.md").write_text("bundled asset\n") + # User skill with metadata.hermes.config SETTINGS (no tags) -> kept. + (base / "skills" / "my-configured").mkdir() + (base / "skills" / "my-configured" / "SKILL.md").write_text( + "---\nname: my-configured\nmetadata:\n hermes:\n" + " config:\n api_url: https://example.com\n---\nBODY\n", + encoding="utf-8") + # User skill with a bare hermes key (no catalog shape) -> kept. + (base / "skills" / "my-bare-meta").mkdir() + (base / "skills" / "my-bare-meta" / "SKILL.md").write_text( + "---\nname: my-bare-meta\nmetadata:\n hermes: {}\n---\nBODY\n", + encoding="utf-8") + collected = spec.collect() + self.assertNotIn("skills/hermes-themes/SKILL.md", collected) + self.assertNotIn("skills/media/heartmula/SKILL.md", collected) + self.assertNotIn("skills/media/heartmula/references/models.md", + collected) + self.assertIn("skills/my-configured/SKILL.md", collected) + self.assertIn("skills/my-bare-meta/SKILL.md", collected) + def test_hermes_collects_hooks_same_framework(self): """hermes collects ``hooks/*`` (lifecycle hooks) for same-framework fidelity, both for the default agent and named agents in all-mode.""" @@ -336,7 +380,10 @@ def test_profiles_are_sub_agents(self): alice = build_spec("openhuman", "Alice", str(self.root)) self.assertEqual(alice.workspace_root, self.ws / "personalities" / "Alice") - self.assertEqual(sorted(alice.collect_bytes()), ["SOUL.md"]) + # Alice lacks IDENTITY.md, so the workspace-level copy falls back in + # (app lookup order: Profile file > workspace-level default). + self.assertEqual(sorted(alice.collect_bytes()), + ["IDENTITY.md", "SOUL.md"]) def test_all_mode_prefixes_profile_dirs(self): spec = build_spec("openhuman", "all", str(self.root)) @@ -359,6 +406,208 @@ def test_fresh_install_without_users_dir_is_not_an_error(self): self.assertEqual(spec.collect_bytes(), {}) +class TestOpenhumanWorkspaceLiveness(unittest.TestCase): + """BUG-0828: reinstalls / user-id migrations leave SEVERAL ``users/`` + dirs behind. The resolver must pick the LIVE workspace by liveness score + (``agent_profiles.json`` / ``personalities/`` / ``SOUL.md``), not the + alphabetically-first one -- a stale ``users/local`` shell used to shadow + the active ``users/local-u-...`` workspace and every convert silently + read the wrong persona. + """ + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) / ".openhuman" + # Stale shell: sorts FIRST, keeps only a persona file. + self.stale = self.root / "users" / "local" / "workspace" + self.stale.mkdir(parents=True) + (self.stale / "SOUL.md").write_text("# stale soul\n") + # Live workspace: machine-generated id sorts AFTER the shell. + self.live = (self.root / "users" / "local-u-x1" / "workspace") + self.live.mkdir(parents=True) + + def tearDown(self): + self.tmp.cleanup() + + def test_live_workspace_beats_stale_sorted_first(self): + (self.live / "agent_profiles.json").write_text( + json.dumps({"activeProfileId": "p"})) + (self.live / "personalities" / "p").mkdir(parents=True) + (self.live / "SOUL.md").write_text("# live soul\n") + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.root, self.live) + self.assertEqual(spec.collect()["SOUL.md"], "# live soul\n") + + def test_personalities_alone_outscores_soul_only_shell(self): + # No agent_profiles.json anywhere: personalities/ (2) + SOUL.md (1) + # still beats the shell's SOUL.md (1). + (self.live / "personalities" / "p").mkdir(parents=True) + (self.live / "SOUL.md").write_text("# live soul\n") + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.root, self.live) + + def test_no_markers_falls_back_to_sorted_first(self): + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.root, self.stale) + + def test_tie_falls_back_to_sorted_first(self): + for ws in (self.stale, self.live): + (ws / "agent_profiles.json").write_text("{}") + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.root, self.stale) + + def test_local_dir_one_level_above_data_root_is_probed(self): + # A backup dir that merely CONTAINS ``.openhuman/`` resolves through. + (self.live / "agent_profiles.json").write_text("{}") + spec = build_spec("openhuman", "default", str(self.root.parent)) + self.assertEqual(spec.root, self.live) + + def test_user_dir_without_workspace_keeps_legacy_path(self): + # users/ exists but has no workspace/ yet: report the canonical + # path anyway (status must not fail on a fresh install). Uses its own + # tree -- setUp's dirs all have workspace/ already. + root = Path(self.tmp.name) / "fresh-oh" / ".openhuman" + no_ws = root / "users" / "aaa" + no_ws.mkdir(parents=True) + spec = build_spec("openhuman", "default", str(root)) + self.assertEqual(spec.root, no_ws / "workspace") + + def test_convert_end_to_end_picks_live_user_active_profile(self): + """Full BUG-0828 shape: stale user sorts first; the LIVE user's + ACTIVE profile carries skills (+openhuman provenance sidecars) but no + SOUL.md of its own. A name-less convert must resolve the live + workspace, pick the active profile, migrate the skills without the + ``_meta.json`` sidecar, and fold the workspace-level SOUL fallback + into the target persona. + """ + profile = self.live / "personalities" / "personalized-agent" + (profile / "skills" / "weather").mkdir(parents=True) + (self.live / "agent_profiles.json").write_text( + json.dumps({"activeProfileId": "personalized-agent"})) + (self.live / "SOUL.md").write_text("# live soul\nGOLD-PERSONA\n") + (profile / "MEMORY.md").write_text("GOLD-MEMORY\n") + (profile / "skills" / "weather" / "SKILL.md").write_text( + "GOLD-WEATHER\n") + (profile / "skills" / "weather" / "_meta.json").write_text('{"k": 1}') + out = Path(self.tmp.name) / "out" + rc = cmd_convert("openhuman", "ms-agent", None, None, + str(self.root), str(out)) + self.assertEqual(rc, 0) + rels = { + str(p.relative_to(out)) for p in out.rglob("*") if p.is_file() + } + self.assertIn("skills/weather/SKILL.md", rels) + self.assertNotIn("skills/weather/_meta.json", rels) + all_text = "".join( + p.read_text(encoding="utf-8") for p in out.rglob("*") + if p.is_file()) + self.assertIn("GOLD-PERSONA", all_text) + self.assertIn("GOLD-MEMORY", all_text) + self.assertIn("GOLD-WEATHER", all_text) + # the stale shell's persona must NOT leak into the output + self.assertNotIn("stale soul", all_text) + + +class TestOpenhumanActiveProfile(unittest.TestCase): + """an omitted --from-name must convert the ACTIVE profile + (``agent_profiles.json`` ``activeProfileId``), not the workspace-level + fallback persona, and a Profile without its own MEMORY.md must fall back + to the workspace-level one -- otherwise converting an openhuman install + loses the active persona's memory. + """ + + USER_ID = "local-u-x" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) / ".openhuman" + self.ws = self.root / "users" / self.USER_ID / "workspace" + librarian = self.ws / "personalities" / "to-ms-librarian" + librarian.mkdir(parents=True) + (self.ws / "personalities" / "idle").mkdir() + # Workspace-level fallback persona + curated memory. + (self.ws / "SOUL.md").write_text("# fallback soul\n") + (self.ws / "IDENTITY.md").write_text("# id\n") + (self.ws / "MEMORY.md").write_text("# workspace memory\n") + # Active profile carries its own SOUL but NOT its own MEMORY. + (librarian / "SOUL.md").write_text("# librarian soul\n") + + def tearDown(self): + self.tmp.cleanup() + + def _write_profiles(self, content): + (self.ws / "agent_profiles.json").write_text(content) + + def test_omitted_name_selects_active_profile(self): + self._write_profiles(json.dumps({"activeProfileId": "to-ms-librarian"})) + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.resolve_default_agent_name(), "to-ms-librarian") + + def test_no_profiles_json_falls_back_to_default(self): + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.resolve_default_agent_name(), "default") + + def test_malformed_profiles_json_falls_back_to_default(self): + self._write_profiles("{not json") + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.resolve_default_agent_name(), "default") + + def test_unknown_profile_id_falls_back_to_default(self): + self._write_profiles(json.dumps({"activeProfileId": "ghost"})) + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.resolve_default_agent_name(), "default") + + def test_empty_profile_id_falls_back_to_default(self): + self._write_profiles(json.dumps({"activeProfileId": " "})) + spec = build_spec("openhuman", "default", str(self.root)) + self.assertEqual(spec.resolve_default_agent_name(), "default") + + def test_profile_memory_falls_back_to_workspace_level(self): + spec = build_spec("openhuman", "to-ms-librarian", str(self.root)) + files = spec.collect() + # Profile's own SOUL wins over the workspace-level one ... + self.assertEqual(files["SOUL.md"], "# librarian soul\n") + # ... while the missing persona files fall back to workspace copies. + self.assertEqual(files["MEMORY.md"], "# workspace memory\n") + self.assertEqual(files["IDENTITY.md"], "# id\n") + self.assertNotIn("config.toml", files) + self.assertNotIn("wiki/note.md", files) + + def test_profile_file_always_wins_over_workspace_fallback(self): + (self.ws / "personalities" / "to-ms-librarian" / + "MEMORY.md").write_text("# profile memory\n") + spec = build_spec("openhuman", "to-ms-librarian", str(self.root)) + self.assertEqual(spec.collect()["MEMORY.md"], "# profile memory\n") + + def test_missing_memory_everywhere_is_silent(self): + (self.ws / "MEMORY.md").unlink() + spec = build_spec("openhuman", "to-ms-librarian", str(self.root)) + self.assertNotIn("MEMORY.md", spec.collect()) + + def test_all_mode_stays_bare_per_profile(self): + spec = build_spec("openhuman", "all", str(self.root)) + # No workspace-level duplication leaks into the per-profile repos. + self.assertEqual(sorted(spec.collect()), + ["to-ms-librarian/SOUL.md"]) + + def test_convert_without_name_uses_active_profile(self): + """cmd_convert auto-selects the active profile end to end.""" + self._write_profiles(json.dumps({"activeProfileId": "to-ms-librarian"})) + out_dir = Path(self.tmp.name) / "out" + rc = cmd_convert( + "openhuman", "nanobot", None, None, str(self.root), str(out_dir)) + self.assertEqual(rc, 0) + # The ACTIVE profile's own SOUL (not the workspace-level fallback + # persona) must be the converted persona ... + self.assertIn("# librarian soul", + (out_dir / "SOUL.md").read_text(encoding="utf-8")) + # ... and the workspace-level MEMORY falls back into the target's + # memory slot instead of being lost (BUG-0825). + memory = (out_dir / "memory" / "MEMORY.md").read_text( + encoding="utf-8") + self.assertIn("# workspace memory", memory) + + class TestQwenpawAgentJsonSecrets(unittest.TestCase): """agent.json sanitize must blank secrets ANYWHERE in the JSON tree.