refactor(manifest): key git config by host
test / unit (pull_request) Successful in 33s
test / integration (pull_request) Successful in 42s

This commit is contained in:
2026-05-28 00:49:34 -04:00
parent 85104742ca
commit 59ee32cc8d
17 changed files with 356 additions and 159 deletions
+15 -13
View File
@@ -42,20 +42,22 @@ def fixture_with_git_dict() -> dict[str, Any]:
return {
"bottles": {
"dev": {
"git": [
{
"Name": "claude-bottle",
"Upstream": "ssh://git@gitea.dideric.is:30009/didericis/claude-bottle.git",
"IdentityFile": "/dev/null",
"KnownHostKey": "ssh-ed25519 AAAA...",
"git": {
"remotes": {
"gitea.dideric.is": {
"Name": "claude-bottle",
"Upstream": "ssh://git@gitea.dideric.is:30009/didericis/claude-bottle.git",
"IdentityFile": "/dev/null",
"KnownHostKey": "ssh-ed25519 AAAA...",
},
"github.com": {
"Name": "foo",
"Upstream": "ssh://git@github.com/didericis/foo.git",
"IdentityFile": "/dev/null",
"KnownHostKey": "ssh-ed25519 BBBB...",
},
},
{
"Name": "foo",
"Upstream": "ssh://git@github.com/didericis/foo.git",
"IdentityFile": "/dev/null",
"KnownHostKey": "ssh-ed25519 BBBB...",
},
]
}
}
},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
+7 -5
View File
@@ -120,11 +120,13 @@ class TestSandboxEscape(unittest.TestCase):
# is intentionally unreachable — the pre-receive
# gitleaks hook must reject BEFORE git-gate
# attempts the upstream push.
"git": [{
"Name": "throwaway",
"Upstream": "ssh://git@unreachable.invalid:22/throwaway.git",
"IdentityFile": str(cls._key_path),
}],
"git": {"remotes": {
"unreachable.invalid": {
"Name": "throwaway",
"Upstream": "ssh://git@unreachable.invalid:22/throwaway.git",
"IdentityFile": str(cls._key_path),
},
}},
},
},
"agents": {
+7 -5
View File
@@ -43,11 +43,13 @@ def _manifest(*, supervise: bool, with_git: bool, with_egress: bool) -> Manifest
if supervise:
bottle["supervise"] = True
if with_git:
bottle["git"] = [{
"Name": "upstream",
"Upstream": "ssh://git@example.com:22/x/y.git",
"IdentityFile": "/etc/hostname", # any existing file
}]
bottle["git"] = {"remotes": {
"example.com": {
"Name": "upstream",
"Upstream": "ssh://git@example.com:22/x/y.git",
"IdentityFile": "/etc/hostname", # any existing file
},
}}
if with_egress:
bottle["egress"] = {
"routes": [{
+1 -1
View File
@@ -26,7 +26,7 @@ def _plan(*, git_user: dict | None = None,
stage_dir: Path | None = None) -> DockerBottlePlan:
bottle_json: dict = {}
if git_user is not None:
bottle_json["git_user"] = git_user
bottle_json["git"] = {"user": git_user}
manifest = Manifest.from_json_obj({
"bottles": {"dev": bottle_json},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
+8 -6
View File
@@ -51,12 +51,14 @@ class TestExtraHostsPlumbing(unittest.TestCase):
m = Manifest.from_json_obj({
"bottles": {
"dev": {
"git": [{
"Name": "claude-bottle",
"Upstream": "ssh://git@gitea.dideric.is:30009/didericis/claude-bottle.git",
"IdentityFile": "/dev/null",
"ExtraHosts": {"gitea.dideric.is": "100.78.141.42"},
}],
"git": {"remotes": {
"gitea.dideric.is": {
"Name": "claude-bottle",
"Upstream": "ssh://git@gitea.dideric.is:30009/didericis/claude-bottle.git",
"IdentityFile": "/dev/null",
"ExtraHosts": {"gitea.dideric.is": "100.78.141.42"},
},
}},
},
},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
+56 -21
View File
@@ -116,10 +116,9 @@ class TestExtendsEnvMerge(unittest.TestCase):
self.assertEqual({"A": "1", "B": "2"}, dict(m.bottles["child"].env))
class TestExtendsListsFullReplace(unittest.TestCase):
"""git: and egress: are full-replace when the child declares
them — partial merge would be ambiguous (ordering + name
collisions). See PRD 0025 "Merge rules"."""
class TestExtendsGitMerge(unittest.TestCase):
"""git.user overlays by field; git.remotes merges by upstream
host, with child entries replacing duplicate hosts."""
_GIT_ENTRY_A = {
"Name": "a",
@@ -132,31 +131,67 @@ class TestExtendsListsFullReplace(unittest.TestCase):
"IdentityFile": "/dev/null",
}
def test_child_git_replaces_parent_entirely(self):
def test_child_git_remotes_merge_with_parent(self):
m = _build(
base={"git": [self._GIT_ENTRY_A]},
child={"extends": "base", "git": [self._GIT_ENTRY_B]},
base={"git": {"remotes": {"host-a": self._GIT_ENTRY_A}}},
child={
"extends": "base",
"git": {"remotes": {"host-b": self._GIT_ENTRY_B}},
},
)
names = [e.Name for e in m.bottles["child"].git]
self.assertEqual(["b"], names)
self.assertEqual(["a", "b"], names)
def test_child_git_remote_replaces_same_host(self):
replacement = {
"Name": "a2",
"Upstream": "ssh://git@host-a/replacement.git",
"IdentityFile": "/dev/null",
}
m = _build(
base={"git": {"remotes": {"host-a": self._GIT_ENTRY_A}}},
child={
"extends": "base",
"git": {"remotes": {"host-a": replacement}},
},
)
entries = m.bottles["child"].git
self.assertEqual(1, len(entries))
self.assertEqual("a2", entries[0].Name)
self.assertEqual("replacement.git", entries[0].UpstreamPath)
def test_child_omits_git_inherits_full_list(self):
m = _build(
base={"git": [self._GIT_ENTRY_A, self._GIT_ENTRY_B]},
base={"git": {"remotes": {
"host-a": self._GIT_ENTRY_A,
"host-b": self._GIT_ENTRY_B,
}}},
child={"extends": "base"},
)
names = [e.Name for e in m.bottles["child"].git]
self.assertEqual(["a", "b"], names)
def test_child_explicit_empty_git_clears_parent(self):
# `git: []` is the documented way to say "drop the
# parent's list" rather than "inherit it".
# `git.remotes: {}` is the documented way to say "drop
# the parent's remotes" rather than "inherit them".
m = _build(
base={"git": [self._GIT_ENTRY_A]},
child={"extends": "base", "git": []},
base={"git": {"remotes": {"host-a": self._GIT_ENTRY_A}}},
child={"extends": "base", "git": {"remotes": {}}},
)
self.assertEqual((), m.bottles["child"].git)
def test_child_git_user_inherits_parent_remotes(self):
m = _build(
base={"git": {"remotes": {"host-a": self._GIT_ENTRY_A}}},
child={"extends": "base", "git": {"user": {"name": "Child"}}},
)
self.assertEqual(["a"], [e.Name for e in m.bottles["child"].git])
self.assertEqual("Child", m.bottles["child"].git_user.name)
class TestExtendsListsFullReplace(unittest.TestCase):
"""egress: remains full-replace when the child declares it."""
def test_child_egress_replaces_parent_entirely(self):
m = _build(
base={"egress": {"routes": [{"host": "a.example.com"}]}},
@@ -178,12 +213,12 @@ class TestExtendsListsFullReplace(unittest.TestCase):
class TestExtendsGitUserOverlay(unittest.TestCase):
"""git_user: per-field overlay. Each non-empty field on child
"""git.user: per-field overlay. Each non-empty field on child
wins; empties fall through to parent."""
def test_parent_full_child_omits(self):
m = _build(
base={"git_user": {"name": "Parent", "email": "p@x"}},
base={"git": {"user": {"name": "Parent", "email": "p@x"}}},
child={"extends": "base"},
)
u = m.bottles["child"].git_user
@@ -192,10 +227,10 @@ class TestExtendsGitUserOverlay(unittest.TestCase):
def test_child_overrides_both(self):
m = _build(
base={"git_user": {"name": "Parent", "email": "p@x"}},
base={"git": {"user": {"name": "Parent", "email": "p@x"}}},
child={
"extends": "base",
"git_user": {"name": "Child", "email": "c@x"},
"git": {"user": {"name": "Child", "email": "c@x"}},
},
)
u = m.bottles["child"].git_user
@@ -206,8 +241,8 @@ class TestExtendsGitUserOverlay(unittest.TestCase):
# Parent sets only name; child sets only email. Both end
# up populated on the child.
m = _build(
base={"git_user": {"name": "Parent"}},
child={"extends": "base", "git_user": {"email": "c@x"}},
base={"git": {"user": {"name": "Parent"}}},
child={"extends": "base", "git": {"user": {"email": "c@x"}}},
)
u = m.bottles["child"].git_user
self.assertEqual("Parent", u.name)
@@ -215,8 +250,8 @@ class TestExtendsGitUserOverlay(unittest.TestCase):
def test_child_overrides_only_email(self):
m = _build(
base={"git_user": {"name": "Parent", "email": "p@x"}},
child={"extends": "base", "git_user": {"email": "c@x"}},
base={"git": {"user": {"name": "Parent", "email": "p@x"}}},
child={"extends": "base", "git": {"user": {"email": "c@x"}}},
)
u = m.bottles["child"].git_user
# Child overrides email; name inherited from parent.
+52 -8
View File
@@ -8,11 +8,26 @@ from claude_bottle.manifest import Manifest
def _manifest(git_entries):
return {
"bottles": {"dev": {"git": git_entries}},
"bottles": {"dev": {"git": {"remotes": {
_host_for(entry): entry for entry in git_entries
}}}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
}
def _host_for(entry):
upstream = entry.get("Upstream", "")
if "@a.example" in upstream:
return "a.example"
if "@b.example" in upstream:
return "b.example"
if "@github.com" in upstream:
return "github.com"
if "@gitea.dideric.is" in upstream:
return "gitea.dideric.is"
return "example.com"
class TestGitEntryParsing(unittest.TestCase):
def test_parses_minimal_entry(self):
m = Manifest.from_json_obj(_manifest([{
@@ -161,12 +176,34 @@ class TestGitEntryExtraHosts(unittest.TestCase):
class TestGitEntryCrossValidation(unittest.TestCase):
def test_duplicate_name_dies(self):
with self.assertRaises(Die):
Manifest.from_json_obj(_manifest([
{"Name": "foo", "Upstream": "ssh://git@a.example/x.git",
"IdentityFile": "/dev/null"},
{"Name": "foo", "Upstream": "ssh://git@b.example/y.git",
"IdentityFile": "/dev/null"},
]))
Manifest.from_json_obj({
"bottles": {"dev": {"git": {"remotes": {
"a.example": {
"Name": "foo",
"Upstream": "ssh://git@a.example/x.git",
"IdentityFile": "/dev/null",
},
"b.example": {
"Name": "foo",
"Upstream": "ssh://git@b.example/y.git",
"IdentityFile": "/dev/null",
},
}}}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
})
def test_remote_key_must_match_upstream_host(self):
with self.assertRaises(Die):
Manifest.from_json_obj({
"bottles": {"dev": {"git": {"remotes": {
"wrong.example": {
"Name": "foo",
"Upstream": "ssh://git@github.com/foo.git",
"IdentityFile": "/dev/null",
},
}}}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
})
def test_legacy_ssh_field_dies_with_hint(self):
# PRD 0009: bottle.ssh is removed; manifests carrying it must
@@ -196,13 +233,20 @@ class TestEmptyGitField(unittest.TestCase):
})
self.assertEqual((), m.bottles["dev"].git)
def test_git_array_type_required(self):
def test_git_object_type_required(self):
with self.assertRaises(Die):
Manifest.from_json_obj({
"bottles": {"dev": {"git": "not-a-list"}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
})
def test_empty_remotes_yields_empty_tuple(self):
m = Manifest.from_json_obj({
"bottles": {"dev": {"git": {"remotes": {}}}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
})
self.assertEqual((), m.bottles["dev"].git)
if __name__ == "__main__":
unittest.main()
+17 -6
View File
@@ -1,4 +1,4 @@
"""Unit: Bottle.git_user manifest parsing + validation (issue #86)."""
"""Unit: Bottle git.user manifest parsing + validation (issue #86)."""
import contextlib
import io
@@ -24,7 +24,7 @@ def _die_message(callable_, *args, **kwargs) -> str:
def _manifest(git_user):
return {
"bottles": {"dev": {"git_user": git_user}},
"bottles": {"dev": {"git": {"user": git_user}}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
}
@@ -53,7 +53,7 @@ class TestGitUserParsing(unittest.TestCase):
self.assertEqual("bot@example.com", u.email)
def test_omitted_defaults_to_empty(self):
# No git_user block at all → empty GitUser, is_empty True →
# No git.user block at all → empty GitUser, is_empty True →
# provisioner skips the `git config` step entirely.
m = Manifest.from_json_obj({
"bottles": {"dev": {}},
@@ -63,7 +63,7 @@ class TestGitUserParsing(unittest.TestCase):
self.assertTrue(u.is_empty())
def test_both_empty_strings_dies(self):
# An explicit `git_user: {name: "", email: ""}` is a typo
# An explicit `git.user: {name: "", email: ""}` is a typo
# / half-finished edit; fail loudly rather than silently
# no-op (the operator clearly meant to configure something).
msg = _die_message(
@@ -83,13 +83,24 @@ class TestGitUserParsing(unittest.TestCase):
msg = _die_message(
Manifest.from_json_obj, _manifest({"name": 42}),
)
self.assertIn("git_user.name must be a string", msg)
self.assertIn("git.user.name must be a string", msg)
def test_non_string_email_dies(self):
msg = _die_message(
Manifest.from_json_obj, _manifest({"email": ["x@y.z"]}),
)
self.assertIn("git_user.email must be a string", msg)
self.assertIn("git.user.email must be a string", msg)
def test_legacy_top_level_git_user_dies(self):
msg = _die_message(
Manifest.from_json_obj,
{
"bottles": {"dev": {"git_user": {"name": "Bot"}}},
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
},
)
self.assertIn("git_user", msg)
self.assertIn("git.user", msg)
class TestGitUserDirect(unittest.TestCase):
+13 -4
View File
@@ -31,6 +31,12 @@ from claude_bottle.pipelock import PipelockProxyPlan
from claude_bottle.supervise import SupervisePlan
def _remote_host(g: GitEntry) -> str:
if g.UpstreamHost:
return g.UpstreamHost
return g.Upstream.split("@", 1)[1].split("/", 1)[0].split(":", 1)[0]
def _plan(
*,
agent_prompt: str = "",
@@ -49,17 +55,20 @@ def _plan(
agent_supervise_url: str = "http://127.0.0.1:55556/",
) -> SmolmachinesBottlePlan:
bottle_json: dict = {}
git_json: dict = {}
if git:
bottle_json["git"] = [
{
git_json["remotes"] = {
_remote_host(g): {
"Name": g.Name,
"Upstream": g.Upstream,
"IdentityFile": g.IdentityFile,
}
for g in git
]
}
if git_user is not None:
bottle_json["git_user"] = git_user
git_json["user"] = git_user
if git_json:
bottle_json["git"] = git_json
if supervise:
bottle_json["supervise"] = True
manifest = Manifest.from_json_obj({
+8 -6
View File
@@ -265,11 +265,13 @@ class TestRealisticBottleFile(unittest.TestCase):
path_allowlist:
- /didericis/
git:
- Name: claude-bottle
Upstream: ssh://git@gitea.dideric.is:30009/x/y.git
IdentityFile: ~/.ssh/gitea.pem
ExtraHosts:
gitea.dideric.is: 100.78.141.42
remotes:
gitea.dideric.is:
Name: claude-bottle
Upstream: ssh://git@gitea.dideric.is:30009/x/y.git
IdentityFile: ~/.ssh/gitea.pem
ExtraHosts:
gitea.dideric.is: 100.78.141.42
""")
# Spot-check the deep parts; the structure is large.
self.assertEqual(2, len(out["egress"]["routes"]))
@@ -283,7 +285,7 @@ class TestRealisticBottleFile(unittest.TestCase):
)
self.assertEqual(
"100.78.141.42",
out["git"][0]["ExtraHosts"]["gitea.dideric.is"],
out["git"]["remotes"]["gitea.dideric.is"]["ExtraHosts"]["gitea.dideric.is"],
)