diff --git a/bot_bottle/backend/docker/bottle_plan.py b/bot_bottle/backend/docker/bottle_plan.py index 2a65c74..8e49c07 100644 --- a/bot_bottle/backend/docker/bottle_plan.py +++ b/bot_bottle/backend/docker/bottle_plan.py @@ -85,6 +85,10 @@ class DockerBottlePlan(BottlePlan): print_multi("skills ", list(agent.skills)) info(f"bottle : {agent.bottle}") + identity = manifest.git_identity_summary(spec.agent_name) + if identity: + info(f" git identity : {identity}") + git_lines = [ f"{u.upstream_host}:{u.upstream_port}" for u in self.git_gate_plan.upstreams diff --git a/bot_bottle/backend/smolmachines/bottle_plan.py b/bot_bottle/backend/smolmachines/bottle_plan.py index 4af4214..e90714a 100644 --- a/bot_bottle/backend/smolmachines/bottle_plan.py +++ b/bot_bottle/backend/smolmachines/bottle_plan.py @@ -125,6 +125,9 @@ class SmolmachinesBottlePlan(BottlePlan): print_multi("env ", env_names) print_multi("skills ", list(agent.skills)) info(f"bottle : {agent.bottle}") + identity = manifest.git_identity_summary(spec.agent_name) + if identity: + info(f" git identity : {identity}") if upstreams: print_multi(" git gate ", upstreams) if routes: diff --git a/bot_bottle/cli/info.py b/bot_bottle/cli/info.py index db74464..9db0faf 100644 --- a/bot_bottle/cli/info.py +++ b/bot_bottle/cli/info.py @@ -31,6 +31,9 @@ def cmd_info(argv: list[str]) -> int: f"first line: {prompt_first_line or '(empty)'}" ) info(f"bottle : {agent.bottle}") + identity = manifest.git_identity_summary(args.name) + if identity: + info(f" git identity : {identity}") if bottle.git: for e in bottle.git: info( diff --git a/bot_bottle/manifest.py b/bot_bottle/manifest.py index fb5d776..430ce17 100644 --- a/bot_bottle/manifest.py +++ b/bot_bottle/manifest.py @@ -47,7 +47,7 @@ import ipaddress import json import os import re -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from pathlib import Path from typing import Mapping, cast @@ -692,6 +692,11 @@ class Agent: bottle: str skills: tuple[str, ...] = () prompt: str = "" + # Per-agent git identity (issue #94). Overlays the referenced + # bottle's git.user per-field at `Manifest.bottle_for`. Only the + # `user` block is allowed at the agent level; `git.remotes` stays + # bottle-only because it carries credentials and host trust. + git_user: GitUser = GitUser() @classmethod def from_dict(cls, name: str, raw: object, bottle_names: set[str]) -> "Agent": @@ -731,7 +736,25 @@ class Agent: else: die(f"agent '{name}' prompt must be a string (was {type(prompt_raw).__name__})") - return cls(bottle=bottle, skills=skills, prompt=prompt) + # git: agents may declare only `git.user` (name/email). Any + # other git key — notably `remotes` — is rejected: remotes + # carry credentials and host trust and stay bottle-only. + git_user = GitUser() + git_raw = d.get("git") + if git_raw is not None: + gd = _as_json_object(git_raw, f"agent '{name}' git") + for k in gd.keys(): + if k != "user": + die( + f"agent '{name}' git.{k} is not allowed at the " + f"agent level; only git.user (name/email) may be " + f"set on an agent. git.remotes is bottle-only " + f"(it carries credentials and host trust)." + ) + if "user" in gd: + git_user = GitUser.from_dict(name, gd["user"]) + + return cls(bottle=bottle, skills=skills, prompt=prompt, git_user=git_user) @dataclass(frozen=True) @@ -874,11 +897,50 @@ class Manifest: ) die(f"bottle '{name}' not defined in bot-bottle.json (no bottles defined).") + def _effective_git_user(self, agent_name: str) -> GitUser: + """Merge the agent's git.user over the referenced bottle's, + per-field, agent-wins-on-non-empty (issue #94). Same overlay + the `extends:` resolver applies between bottles + (`_merge_bottles`).""" + agent = self.agents[agent_name] + base = self.bottles[agent.bottle].git_user + over = agent.git_user + if over.is_empty(): + return base + return GitUser( + name=over.name or base.name, + email=over.email or base.email, + ) + def bottle_for(self, agent_name: str) -> Bottle: - """Resolve the Bottle the named agent references. The validator - guarantees both lookups succeed for a manifest built via - from_json_obj.""" - return self.bottles[self.agents[agent_name].bottle] + """Resolve the Bottle the named agent references, with the + agent's git.user overlaid on top. The validator guarantees both + lookups succeed for a manifest built via from_json_obj. + + The overlay lives here, the single point both backends call to + resolve an agent's bottle, so the docker / smolmachines git + provisioners pick up the merged identity unchanged.""" + bottle = self.bottles[self.agents[agent_name].bottle] + merged = self._effective_git_user(agent_name) + if merged == bottle.git_user: + return bottle + return replace(bottle, git_user=merged) + + def git_identity_summary(self, agent_name: str) -> str | None: + """One-line effective git identity with per-field provenance + for launch summaries, e.g. + `name=claude (agent), email=eric@dideric.is (bottle)`. + Returns None when neither agent nor bottle sets an identity.""" + over = self.agents[agent_name].git_user + merged = self._effective_git_user(agent_name) + if merged.is_empty(): + return None + parts: list[str] = [] + if merged.name: + parts.append(f"name={merged.name} ({'agent' if over.name else 'bottle'})") + if merged.email: + parts.append(f"email={merged.email} ({'agent' if over.email else 'bottle'})") + return ", ".join(parts) def _as_json_object(value: object, label: str) -> dict[str, object]: @@ -1053,7 +1115,7 @@ _BOTTLE_KEYS = frozenset( {"env", "extends", "agent_provider", "git", "egress", "supervise"} ) _AGENT_KEYS_REQUIRED = frozenset({"bottle"}) -_AGENT_KEYS_OPTIONAL = frozenset({"skills"}) +_AGENT_KEYS_OPTIONAL = frozenset({"skills", "git"}) # Claude Code subagent fields bot-bottle ignores at launch but # doesn't reject — lets the same file double as `~/.claude/agents/*.md`. _AGENT_KEYS_CC_PASSTHROUGH = frozenset({ @@ -1301,11 +1363,13 @@ def _load_agents_from_dir( ) # Build the dict Agent.from_dict expects. The body becomes # prompt; CC passthrough fields stay in fm and get ignored - # by from_dict (which only reads bottle/skills/prompt). + # by from_dict (which reads bottle/skills/git/prompt). agent_dict: dict[str, object] = { "bottle": fm.get("bottle"), "skills": fm.get("skills", []), "prompt": body.strip(), } + if "git" in fm: + agent_dict["git"] = fm["git"] out[name] = Agent.from_dict(name, agent_dict, bottle_names) return out diff --git a/tests/unit/test_manifest_agent_git_user.py b/tests/unit/test_manifest_agent_git_user.py new file mode 100644 index 0000000..887b161 --- /dev/null +++ b/tests/unit/test_manifest_agent_git_user.py @@ -0,0 +1,248 @@ +"""Unit: agent-level git.user overlay + provenance (PRD 0027, issue #94). + +An agent file may declare `git.user` (name/email). At +`Manifest.bottle_for()` it overlays the referenced bottle's +`git.user` per-field, agent-wins-on-non-empty. `git.remotes` is +rejected on agents. `Manifest.git_identity_summary()` reports the +effective identity with per-field `(agent)`/`(bottle)` provenance. + +The `from_json_obj` path drives `Agent.from_dict` + `bottle_for`; +a temp-dir case locks the md loader (the `_AGENT_KEYS` allow + the +`git` threading into `agent_dict`).""" + +from __future__ import annotations + +import contextlib +import io +import os +import shutil +import tempfile +import textwrap +import unittest +from pathlib import Path + +from bot_bottle.log import Die +from bot_bottle.manifest import Manifest + + +def _die_message(callable_, *args, **kwargs) -> str: + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + try: + callable_(*args, **kwargs) + except Die: + return buf.getvalue() + raise AssertionError("expected Die was not raised") + + +def _manifest(*, bottle_user=None, agent_git=None) -> Manifest: + bottle: dict = {} + if bottle_user is not None: + bottle = {"git": {"user": bottle_user}} + agent: dict = {"skills": [], "prompt": "", "bottle": "dev"} + if agent_git is not None: + agent["git"] = agent_git + return Manifest.from_json_obj({ + "bottles": {"dev": bottle}, + "agents": {"impl": agent}, + }) + + +class TestAgentGitUserOverlay(unittest.TestCase): + def test_agent_supplies_both_fields(self): + m = _manifest(agent_git={"user": {"name": "a", "email": "a@b"}}) + u = m.bottle_for("impl").git_user + self.assertEqual("a", u.name) + self.assertEqual("a@b", u.email) + + def test_agent_name_only_email_falls_through_to_bottle(self): + m = _manifest( + bottle_user={"name": "B", "email": "b@c"}, + agent_git={"user": {"name": "a"}}, + ) + u = m.bottle_for("impl").git_user + self.assertEqual("a", u.name) # agent wins + self.assertEqual("b@c", u.email) # bottle falls through + + def test_agent_email_only_name_falls_through_to_bottle(self): + m = _manifest( + bottle_user={"name": "B", "email": "b@c"}, + agent_git={"user": {"email": "a@b"}}, + ) + u = m.bottle_for("impl").git_user + self.assertEqual("B", u.name) + self.assertEqual("a@b", u.email) + + def test_agent_identity_with_bottle_declaring_none(self): + m = _manifest(agent_git={"user": {"name": "a", "email": "a@b"}}) + # The underlying bottle declares no identity; the merged one does. + self.assertTrue(m.bottles["dev"].git_user.is_empty()) + self.assertFalse(m.bottle_for("impl").git_user.is_empty()) + + def test_bottle_only_identity_preserved_when_agent_silent(self): + m = _manifest(bottle_user={"name": "B", "email": "b@c"}) + u = m.bottle_for("impl").git_user + self.assertEqual("B", u.name) + self.assertEqual("b@c", u.email) + + def test_bottle_for_returns_same_instance_when_no_overlay(self): + # No agent git.user → no replace(); the cached Bottle is + # returned as-is (identity check guards against churn). + m = _manifest(bottle_user={"name": "B"}) + self.assertIs(m.bottles["dev"], m.bottle_for("impl")) + + def test_bottle_for_returns_same_instance_when_overlay_is_noop(self): + # Agent restates exactly what the bottle already has → merged + # == bottle.git_user → same instance, no replace(). + m = _manifest( + bottle_user={"name": "B", "email": "b@c"}, + agent_git={"user": {"name": "B", "email": "b@c"}}, + ) + self.assertIs(m.bottles["dev"], m.bottle_for("impl")) + + def test_other_bottle_fields_untouched_by_overlay(self): + m = Manifest.from_json_obj({ + "bottles": {"dev": { + "env": {"FOO": "bar"}, + "supervise": True, + "git": {"user": {"name": "B"}}, + }}, + "agents": {"impl": { + "bottle": "dev", "skills": [], "prompt": "", + "git": {"user": {"name": "a"}}, + }}, + }) + b = m.bottle_for("impl") + self.assertEqual("a", b.git_user.name) + self.assertEqual({"FOO": "bar"}, dict(b.env)) + self.assertTrue(b.supervise) + + +class TestAgentGitUserRejections(unittest.TestCase): + def test_agent_remotes_dies_bottle_only(self): + msg = _die_message(_manifest, agent_git={ + "remotes": {"h": {"Name": "r", "Upstream": "ssh://x/y.git"}}, + }) + self.assertIn("git.remotes", msg) + self.assertIn("bottle-only", msg) + + def test_agent_unknown_git_subkey_dies(self): + msg = _die_message(_manifest, agent_git={"nope": {}}) + self.assertIn("not allowed at the agent level", msg) + + def test_agent_git_user_both_empty_dies(self): + # Reuses GitUser.from_dict validation. + msg = _die_message(_manifest, agent_git={"user": {"name": "", "email": ""}}) + self.assertIn("neither name nor email", msg) + + +class TestGitIdentitySummary(unittest.TestCase): + def test_both_from_agent(self): + m = _manifest(agent_git={"user": {"name": "a", "email": "a@b"}}) + self.assertEqual( + "name=a (agent), email=a@b (agent)", + m.git_identity_summary("impl"), + ) + + def test_mixed_provenance(self): + m = _manifest( + bottle_user={"name": "B", "email": "b@c"}, + agent_git={"user": {"name": "a"}}, + ) + self.assertEqual( + "name=a (agent), email=b@c (bottle)", + m.git_identity_summary("impl"), + ) + + def test_bottle_only(self): + m = _manifest(bottle_user={"name": "B", "email": "b@c"}) + self.assertEqual( + "name=B (bottle), email=b@c (bottle)", + m.git_identity_summary("impl"), + ) + + def test_none_when_unset_anywhere(self): + m = _manifest() + self.assertIsNone(m.git_identity_summary("impl")) + + +_BOTTLE_DEV = """ + --- + git: + user: + name: bottle-name + email: bottle@example.com + --- + + dev bottle. +""" + +_AGENT_WITH_GIT = """ + --- + bottle: dev + git: + user: + name: agent-name + --- + + impl agent. +""" + +_AGENT_WITH_REMOTES = """ + --- + bottle: dev + git: + remotes: + h: + Name: r + Upstream: ssh://x/y.git + --- + + bad agent. +""" + + +class TestAgentGitUserMdLoader(unittest.TestCase): + """Locks the md path: `git` is an accepted agent key and threads + into the parsed Agent (not rejected as an unknown frontmatter + key), and agent `git.remotes` dies through the same loader.""" + + def setUp(self) -> None: + self.home = Path(tempfile.mkdtemp(prefix="cb-home-")) + self._orig_home = os.environ.get("HOME") + os.environ["HOME"] = str(self.home) + + def tearDown(self) -> None: + if self._orig_home is None: + os.environ.pop("HOME", None) + else: + os.environ["HOME"] = self._orig_home + shutil.rmtree(self.home, ignore_errors=True) + + def _write(self, rel: str, text: str) -> None: + p = self.home / ".bot-bottle" / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(textwrap.dedent(text).lstrip("\n")) + + def test_md_agent_git_user_overlays_bottle(self): + self._write("bottles/dev.md", _BOTTLE_DEV) + self._write("agents/impl.md", _AGENT_WITH_GIT) + m = Manifest.resolve(str(self.home)) + u = m.bottle_for("impl").git_user + self.assertEqual("agent-name", u.name) # agent wins + self.assertEqual("bottle@example.com", u.email) # bottle falls through + self.assertEqual( + "name=agent-name (agent), email=bottle@example.com (bottle)", + m.git_identity_summary("impl"), + ) + + def test_md_agent_remotes_dies(self): + self._write("bottles/dev.md", _BOTTLE_DEV) + self._write("agents/impl.md", _AGENT_WITH_REMOTES) + msg = _die_message(Manifest.resolve, str(self.home)) + self.assertIn("git.remotes", msg) + self.assertIn("bottle-only", msg) + + +if __name__ == "__main__": + unittest.main()