Compare commits

..

5 Commits

Author SHA1 Message Date
didericis 65faa40b9a refactor(backend): remove _validate_git_entries host key-file check
test / unit (pull_request) Successful in 37s
test / integration (pull_request) Successful in 18s
lint / lint (push) Successful in 1m39s
test / unit (push) Successful in 37s
test / integration (push) Successful in 18s
Update Quality Badges / update-badges (push) Successful in 1m38s
The git-gate copies the identity file at start time and surfaces a
clear failure then; the pre-launch presence check was redundant.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-22 14:44:46 -04:00
didericis 9f97de115b fix(git-gate): skip host key-file check for gitea-provider repos
lint / lint (push) Successful in 1m39s
test / unit (pull_request) Successful in 34s
test / integration (pull_request) Successful in 18s
_validate_git_entries was written for static keys (PRD 0008) and ran
os.path.isfile() on every entry's IdentityFile. gitea-provider repos
(PRD 0047/0048) create their deploy key at provision time, so
IdentityFile is empty at parse — tripping the check with an empty path
("git upstream key file not found for '<name>': "). Gate the host-file
check on the static provider; gitea entries have nothing to verify here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-22 13:21:11 -04:00
didericis 8f21f4df19 refactor(manifest-extends): thread resolved repos through recursion
lint / lint (push) Successful in 1m32s
test / unit (pull_request) Successful in 28s
test / integration (pull_request) Successful in 16s
Replace the lossy _entry_to_raw round-trip with a repos_cache threaded
alongside the ManifestBottle cache in _resolve_one_bottle. Each bottle's
effective git-gate.repos is stored as raw dicts keyed by name, so a child
field-merges directly against its parent's raw repos instead of
reconstructing them from parsed ManifestGitEntry objects.

_resolve_repos_raw now owns the union/clear/inherit semantics on plain
dicts; _merge_bottles just injects the precomputed merged set before
parsing. Drops _entry_to_raw entirely, removing the maintenance hazard
where a new ManifestGitEntry field would silently vanish from inherited
repos.

Addresses review feedback on #238.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NgEFTXcWZjA8n7ntq2zHQQ
2026-06-19 22:53:27 -04:00
didericis-claude ff7a52c1d2 refactor(manifest-extends): simplify git-gate repo merge to union + dict unpack
lint / lint (push) Failing after 1m31s
test / unit (pull_request) Successful in 30s
test / integration (pull_request) Successful in 17s
Replace the bespoke _pre_merge_git_repos loop and _merge_git_remotes
with a single _merge_git_repos_raw that does a name-keyed union merge
at the raw dict level: build parent_repos from _entry_to_raw, then
for each name in set(child) | set(parent) produce {**parent.get(n,{}),
**child.get(n,{})}. child.git after from_dict already has the full
merged set, so _merge_git_remotes is no longer needed.
2026-06-20 02:25:09 +00:00
didericis-claude 4ed6b84863 feat(manifest-extends): field-merge same-name git-gate repos on extends
lint / lint (push) Successful in 1m34s
test / unit (pull_request) Successful in 27s
test / integration (pull_request) Successful in 15s
When a child bottle declares a git-gate repo with the same name as a
parent repo, merge field-by-field (child wins, parent provides fallback)
instead of letting the child entry silently replace the parent entry.
This lets a child override only `key:` without repeating `url:` and
`host_key:`. Change the merge key in _merge_git_remotes from UpstreamHost
to Name, which is the natural unique identity for a repo entry.

Closes #237
2026-06-20 02:02:12 +00:00
4 changed files with 155 additions and 47 deletions
+5 -17
View File
@@ -45,7 +45,7 @@ from ..agent_provider import AgentProvisionPlan, get_provider, build_agent_provi
from ..egress import EgressPlan
from ..git_gate import GitGatePlan
from ..log import die, info
from ..manifest import ManifestGitEntry, Manifest
from ..manifest import Manifest
from ..supervise import SupervisePlan
from ..util import expand_tilde
from ..env import resolve_env, ResolvedEnv
@@ -356,16 +356,14 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
pass
def _validate(self, spec: BottleSpec) -> None:
"""Cross-backend pre-launch checks. Confirms the agent exists,
the named skills are present on the host, and every git
IdentityFile resolves. Subclasses with additional preconditions
should override and call `super()._validate(spec)` first."""
"""Cross-backend pre-launch checks. Confirms the agent exists
and the named skills are present on the host. Subclasses with
additional preconditions should override and call
`super()._validate(spec)` first."""
manifest = spec.manifest
manifest.require_agent(spec.agent_name)
agent = manifest.agents[spec.agent_name]
bottle = manifest.bottle_for(spec.agent_name)
self._validate_skills(agent.skills)
self._validate_git_entries(bottle.git)
self._validate_agent_provider_dockerfile(spec)
def _validate_skills(self, skills: Sequence[str]) -> None:
@@ -380,16 +378,6 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
f"Create it under ~/.claude/skills/, then re-run."
)
def _validate_git_entries(self, entries: Sequence[ManifestGitEntry]) -> None:
"""Each entry's IdentityFile must exist on the host (after
expanding leading ~) — the git-gate copies it in at start time
to authenticate the upstream push (PRD 0008). Shape is already
enforced by Manifest validation; this only checks presence."""
for entry in entries:
key = expand_tilde(entry.IdentityFile)
if not os.path.isfile(key):
die(f"git upstream key file not found for '{entry.Name}': {key}")
def _validate_agent_provider_dockerfile(self, spec: BottleSpec) -> None:
bottle = spec.manifest.bottle_for(spec.agent_name)
dockerfile = bottle.agent_provider.dockerfile
+66 -18
View File
@@ -5,16 +5,20 @@ from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from .manifest import ManifestBottle, ManifestGitEntry
from .manifest import ManifestBottle
from .manifest_egress import ManifestEgressConfig
def resolve_bottles(raws: dict[str, dict[str, object]]) -> dict[str, ManifestBottle]:
"""Apply `extends:` chains and return resolved ManifestBottle objects."""
cache: dict[str, ManifestBottle] = {}
# Per-bottle effective git-gate.repos, as raw dicts keyed by repo name.
# Threaded alongside `cache` so a child can field-merge against its
# parent's repos without reconstructing them from parsed entries.
repos_cache: dict[str, dict[str, object]] = {}
for name in raws:
if name not in cache:
_resolve_one_bottle(name, raws, cache, ())
_resolve_one_bottle(name, raws, cache, repos_cache, ())
return cache
@@ -22,6 +26,7 @@ def _resolve_one_bottle(
name: str,
raws: dict[str, dict[str, object]],
cache: dict[str, ManifestBottle],
repos_cache: dict[str, dict[str, object]],
seen: tuple[str, ...],
) -> ManifestBottle:
from .manifest import ManifestBottle, ManifestError
@@ -41,6 +46,7 @@ def _resolve_one_bottle(
if parent_name_raw is None:
bottle = ManifestBottle.from_dict(name, child_raw)
cache[name] = bottle
repos_cache[name] = _resolve_repos_raw({}, child_raw)
return bottle
if not isinstance(parent_name_raw, str):
@@ -60,20 +66,33 @@ def _resolve_one_bottle(
f"bottle '{name}' extends '{parent_name}' which is not "
f"defined. Available bottles: {avail}"
)
parent = _resolve_one_bottle(parent_name, raws, cache, seen + (name,))
bottle = _merge_bottles(parent, child_raw, name)
parent = _resolve_one_bottle(
parent_name, raws, cache, repos_cache, seen + (name,)
)
merged_repos_raw = _resolve_repos_raw(repos_cache[parent_name], child_raw)
bottle = _merge_bottles(parent, child_raw, merged_repos_raw, name)
cache[name] = bottle
repos_cache[name] = merged_repos_raw
return bottle
def _merge_bottles(
parent: ManifestBottle,
child_raw: dict[str, object],
merged_repos_raw: dict[str, object],
name: str,
) -> ManifestBottle:
"""Apply PRD 0025 merge rules."""
from .manifest import ManifestBottle, ManifestGitUser
from .manifest_egress import validate_egress_routes
from .manifest_util import as_json_object
# git-gate.repos: when the child declares repos, inject the already
# name-merged repo set (computed by _resolve_repos_raw) so the child
# parses with the full inherited+overridden list (issue #237).
if _child_declares_git_gate_repos(child_raw):
git_raw = as_json_object(child_raw.get("git-gate", {}), "child git-gate")
child_raw = {**child_raw, "git-gate": {**git_raw, "repos": merged_repos_raw}}
# Parse the child's declared fields into a ManifestBottle (with the
# usual defaults for anything missing). Validation runs the same
@@ -92,11 +111,11 @@ def _merge_bottles(
email=child.git_user.email or parent.git_user.email,
)
# git-gate.repos: missing means inherit; an explicit empty object
# clears; otherwise parent and child merge by UpstreamHost with
# child entries replacing duplicate hosts.
# git-gate.repos: when declared, child.git already holds the merged
# set (an explicit empty dict clears parent, leaving child.git empty).
# When omitted, the parent's entries are inherited verbatim.
if _child_declares_git_gate_repos(child_raw):
merged_git = _merge_git_remotes(parent.git, child.git) if child.git else ()
merged_git = child.git
else:
merged_git = parent.git
@@ -130,6 +149,45 @@ def _merge_bottles(
)
def _resolve_repos_raw(
parent_repos: dict[str, object],
child_raw: dict[str, object],
) -> dict[str, object]:
"""Compute a bottle's effective git-gate.repos as raw dicts.
Repos are keyed by name. When the child omits git-gate.repos it
inherits the parent's set verbatim; an explicit empty dict clears it.
Otherwise parent and child unite by name, with same-name entries
field-merged (parent fields are defaults, child fields win)."""
from .manifest_util import as_json_object
if not _child_declares_git_gate_repos(child_raw):
return parent_repos
child_repos = _declared_repos_raw(child_raw)
if not child_repos:
return {}
# Parent entries keep their order; child-only names are appended.
names = list(parent_repos) + [n for n in child_repos if n not in parent_repos]
return {
name: {
**as_json_object(parent_repos.get(name, {}), "parent git-gate repo"),
**as_json_object(child_repos.get(name, {}), "child git-gate repo"),
}
for name in names
}
def _declared_repos_raw(child_raw: dict[str, object]) -> dict[str, object]:
"""Return the child's explicitly declared git-gate.repos as raw dicts,
or an empty dict when none are declared."""
from .manifest_util import as_json_object
if not _child_declares_git_gate_repos(child_raw):
return {}
git_raw = as_json_object(child_raw.get("git-gate", {}), "child git-gate")
return as_json_object(git_raw.get("repos", {}), "child git-gate.repos")
def _child_declares_git_gate_repos(child_raw: dict[str, object]) -> bool:
from .manifest_util import as_json_object
@@ -140,16 +198,6 @@ def _child_declares_git_gate_repos(child_raw: dict[str, object]) -> bool:
return "repos" in git_obj
def _merge_git_remotes(
parent: tuple[ManifestGitEntry, ...],
child: tuple[ManifestGitEntry, ...],
) -> tuple[ManifestGitEntry, ...]:
by_host = {entry.UpstreamHost: entry for entry in parent}
for entry in child:
by_host[entry.UpstreamHost] = entry
return tuple(by_host.values())
def _merge_egress(
parent: ManifestEgressConfig,
child: ManifestEgressConfig,
+3 -4
View File
@@ -92,10 +92,9 @@ class TestSandboxEscape(unittest.TestCase):
"on PATH: curl -sSL https://smolmachines.com/install.sh | sh"
)
# Throwaway "identity file" so the manifest's _validate_git_entries
# passes (it only checks `os.path.isfile`, not that the content is
# a real SSH key). Test 5 reaches gitleaks before any SSH attempt
# anyway.
# Throwaway "identity file" for the git-gate's `identity` field.
# It need not be a real SSH key: test 5 reaches gitleaks before
# any SSH attempt anyway.
fd, kp = tempfile.mkstemp(prefix="sandbox-test-key.")
os.close(fd)
cls._key_path = Path(kp)
+81 -8
View File
@@ -113,8 +113,8 @@ class TestExtendsEnvMerge(unittest.TestCase):
class TestExtendsGitMerge(unittest.TestCase):
"""git-gate.user overlays by field; git-gate.repos merges by upstream
host, with child entries replacing duplicate hosts."""
"""git-gate.user overlays by field; git-gate.repos merges by name,
with same-name child entries merging field-by-field (child wins)."""
_GIT_ENTRY_A = {"url": "ssh://git@host-a/a.git", "key": {"provider": "static", "path": "/dev/null"}}
_GIT_ENTRY_B = {"url": "ssh://git@host-b/b.git", "key": {"provider": "static", "path": "/dev/null"}}
@@ -130,19 +130,21 @@ class TestExtendsGitMerge(unittest.TestCase):
names = [e.Name for e in m.bottles["child"].git]
self.assertEqual(["a", "b"], names)
def test_child_git_repo_replaces_same_host(self):
replacement = {"url": "ssh://git@host-a/replacement.git", "key": {"provider": "static", "path": "/dev/null"}}
def test_child_git_repo_different_name_same_host_coexists(self):
# Repos are keyed by Name, not UpstreamHost: two repos with
# different names on the same host both survive the merge.
same_host_b = {"url": "ssh://git@host-a/b.git", "key": {"provider": "static", "path": "/dev/null"}}
m = _build(
base={"git-gate": {"repos": {"a": self._GIT_ENTRY_A}}},
child={
"extends": "base",
"git-gate": {"repos": {"a2": replacement}},
"git-gate": {"repos": {"a2": same_host_b}},
},
)
entries = m.bottles["child"].git
self.assertEqual(1, len(entries))
self.assertEqual("a2", entries[0].Name)
self.assertEqual("replacement.git", entries[0].UpstreamPath)
self.assertEqual(2, len(entries))
names = {e.Name for e in entries}
self.assertEqual({"a", "a2"}, names)
def test_child_omits_git_gate_inherits_full_list(self):
m = _build(
@@ -164,6 +166,77 @@ class TestExtendsGitMerge(unittest.TestCase):
)
self.assertEqual((), m.bottles["child"].git)
def test_child_same_name_repo_merges_key_field(self):
# Issue #237: child repo with same name as parent should merge
# field-by-field. Child overrides only `key`; parent's url and
# host_key are preserved.
parent_entry = {
"url": "ssh://git@host-a/repo.git",
"host_key": "ecdsa-sha2-nistp256 AAAA",
"key": {"provider": "static", "path": "/keys/id_rsa"},
}
m = _build(
base={"git-gate": {"repos": {"repo": parent_entry}}},
child={
"extends": "base",
"git-gate": {"repos": {"repo": {
"key": {"provider": "gitea", "forge_token_env": "GITEA_TOKEN"},
}}},
},
)
entries = m.bottles["child"].git
self.assertEqual(1, len(entries))
e = entries[0]
self.assertEqual("repo", e.Name)
self.assertEqual("ssh://git@host-a/repo.git", e.Upstream)
self.assertEqual("ecdsa-sha2-nistp256 AAAA", e.KnownHostKey)
self.assertEqual("gitea", e.Key.provider)
self.assertEqual("GITEA_TOKEN", e.Key.forge_token_env)
def test_child_same_name_repo_overrides_url(self):
# Child can override url on a same-name repo; other parent fields
# fall through.
parent_entry = {
"url": "ssh://git@host-a/old.git",
"key": {"provider": "static", "path": "/keys/id_rsa"},
}
m = _build(
base={"git-gate": {"repos": {"repo": parent_entry}}},
child={
"extends": "base",
"git-gate": {"repos": {"repo": {
"url": "ssh://git@host-b/new.git",
"key": {"provider": "static", "path": "/keys/id_rsa"},
}}},
},
)
entries = m.bottles["child"].git
self.assertEqual(1, len(entries))
self.assertEqual("ssh://git@host-b/new.git", entries[0].Upstream)
def test_child_same_name_plus_new_repo(self):
# Same-name repo is field-merged; a distinct new name in child
# is appended.
parent_entry = {
"url": "ssh://git@host-a/repo.git",
"key": {"provider": "static", "path": "/keys/id_rsa"},
}
m = _build(
base={"git-gate": {"repos": {"repo": parent_entry}}},
child={
"extends": "base",
"git-gate": {"repos": {
"repo": {"key": {"provider": "gitea", "forge_token_env": "TOK"}},
"other": self._GIT_ENTRY_B,
}},
},
)
child = m.bottles["child"]
names = {e.Name for e in child.git}
self.assertEqual({"repo", "other"}, names)
repo_entry = next(e for e in child.git if e.Name == "repo")
self.assertEqual("gitea", repo_entry.Key.provider)
def test_child_git_user_inherits_parent_repos(self):
m = _build(
base={"git-gate": {"repos": {"a": self._GIT_ENTRY_A}}},