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 ..egress import EgressPlan
from ..git_gate import GitGatePlan from ..git_gate import GitGatePlan
from ..log import die, info from ..log import die, info
from ..manifest import ManifestGitEntry, Manifest from ..manifest import Manifest
from ..supervise import SupervisePlan from ..supervise import SupervisePlan
from ..util import expand_tilde from ..util import expand_tilde
from ..env import resolve_env, ResolvedEnv from ..env import resolve_env, ResolvedEnv
@@ -356,16 +356,14 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
pass pass
def _validate(self, spec: BottleSpec) -> None: def _validate(self, spec: BottleSpec) -> None:
"""Cross-backend pre-launch checks. Confirms the agent exists, """Cross-backend pre-launch checks. Confirms the agent exists
the named skills are present on the host, and every git and the named skills are present on the host. Subclasses with
IdentityFile resolves. Subclasses with additional preconditions additional preconditions should override and call
should override and call `super()._validate(spec)` first.""" `super()._validate(spec)` first."""
manifest = spec.manifest manifest = spec.manifest
manifest.require_agent(spec.agent_name) manifest.require_agent(spec.agent_name)
agent = manifest.agents[spec.agent_name] agent = manifest.agents[spec.agent_name]
bottle = manifest.bottle_for(spec.agent_name)
self._validate_skills(agent.skills) self._validate_skills(agent.skills)
self._validate_git_entries(bottle.git)
self._validate_agent_provider_dockerfile(spec) self._validate_agent_provider_dockerfile(spec)
def _validate_skills(self, skills: Sequence[str]) -> None: 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." 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: def _validate_agent_provider_dockerfile(self, spec: BottleSpec) -> None:
bottle = spec.manifest.bottle_for(spec.agent_name) bottle = spec.manifest.bottle_for(spec.agent_name)
dockerfile = bottle.agent_provider.dockerfile dockerfile = bottle.agent_provider.dockerfile
+66 -18
View File
@@ -5,16 +5,20 @@ from __future__ import annotations
from typing import TYPE_CHECKING from typing import TYPE_CHECKING
if TYPE_CHECKING: if TYPE_CHECKING:
from .manifest import ManifestBottle, ManifestGitEntry from .manifest import ManifestBottle
from .manifest_egress import ManifestEgressConfig from .manifest_egress import ManifestEgressConfig
def resolve_bottles(raws: dict[str, dict[str, object]]) -> dict[str, ManifestBottle]: def resolve_bottles(raws: dict[str, dict[str, object]]) -> dict[str, ManifestBottle]:
"""Apply `extends:` chains and return resolved ManifestBottle objects.""" """Apply `extends:` chains and return resolved ManifestBottle objects."""
cache: dict[str, ManifestBottle] = {} 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: for name in raws:
if name not in cache: if name not in cache:
_resolve_one_bottle(name, raws, cache, ()) _resolve_one_bottle(name, raws, cache, repos_cache, ())
return cache return cache
@@ -22,6 +26,7 @@ def _resolve_one_bottle(
name: str, name: str,
raws: dict[str, dict[str, object]], raws: dict[str, dict[str, object]],
cache: dict[str, ManifestBottle], cache: dict[str, ManifestBottle],
repos_cache: dict[str, dict[str, object]],
seen: tuple[str, ...], seen: tuple[str, ...],
) -> ManifestBottle: ) -> ManifestBottle:
from .manifest import ManifestBottle, ManifestError from .manifest import ManifestBottle, ManifestError
@@ -41,6 +46,7 @@ def _resolve_one_bottle(
if parent_name_raw is None: if parent_name_raw is None:
bottle = ManifestBottle.from_dict(name, child_raw) bottle = ManifestBottle.from_dict(name, child_raw)
cache[name] = bottle cache[name] = bottle
repos_cache[name] = _resolve_repos_raw({}, child_raw)
return bottle return bottle
if not isinstance(parent_name_raw, str): 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"bottle '{name}' extends '{parent_name}' which is not "
f"defined. Available bottles: {avail}" f"defined. Available bottles: {avail}"
) )
parent = _resolve_one_bottle(parent_name, raws, cache, seen + (name,)) parent = _resolve_one_bottle(
bottle = _merge_bottles(parent, child_raw, name) 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 cache[name] = bottle
repos_cache[name] = merged_repos_raw
return bottle return bottle
def _merge_bottles( def _merge_bottles(
parent: ManifestBottle, parent: ManifestBottle,
child_raw: dict[str, object], child_raw: dict[str, object],
merged_repos_raw: dict[str, object],
name: str, name: str,
) -> ManifestBottle: ) -> ManifestBottle:
"""Apply PRD 0025 merge rules.""" """Apply PRD 0025 merge rules."""
from .manifest import ManifestBottle, ManifestGitUser from .manifest import ManifestBottle, ManifestGitUser
from .manifest_egress import validate_egress_routes 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 # Parse the child's declared fields into a ManifestBottle (with the
# usual defaults for anything missing). Validation runs the same # 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, email=child.git_user.email or parent.git_user.email,
) )
# git-gate.repos: missing means inherit; an explicit empty object # git-gate.repos: when declared, child.git already holds the merged
# clears; otherwise parent and child merge by UpstreamHost with # set (an explicit empty dict clears parent, leaving child.git empty).
# child entries replacing duplicate hosts. # When omitted, the parent's entries are inherited verbatim.
if _child_declares_git_gate_repos(child_raw): 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: else:
merged_git = parent.git 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: def _child_declares_git_gate_repos(child_raw: dict[str, object]) -> bool:
from .manifest_util import as_json_object 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 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( def _merge_egress(
parent: ManifestEgressConfig, parent: ManifestEgressConfig,
child: 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" "on PATH: curl -sSL https://smolmachines.com/install.sh | sh"
) )
# Throwaway "identity file" so the manifest's _validate_git_entries # Throwaway "identity file" for the git-gate's `identity` field.
# passes (it only checks `os.path.isfile`, not that the content is # It need not be a real SSH key: test 5 reaches gitleaks before
# a real SSH key). Test 5 reaches gitleaks before any SSH attempt # any SSH attempt anyway.
# anyway.
fd, kp = tempfile.mkstemp(prefix="sandbox-test-key.") fd, kp = tempfile.mkstemp(prefix="sandbox-test-key.")
os.close(fd) os.close(fd)
cls._key_path = Path(kp) cls._key_path = Path(kp)
+81 -8
View File
@@ -113,8 +113,8 @@ class TestExtendsEnvMerge(unittest.TestCase):
class TestExtendsGitMerge(unittest.TestCase): class TestExtendsGitMerge(unittest.TestCase):
"""git-gate.user overlays by field; git-gate.repos merges by upstream """git-gate.user overlays by field; git-gate.repos merges by name,
host, with child entries replacing duplicate hosts.""" 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_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"}} _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] names = [e.Name for e in m.bottles["child"].git]
self.assertEqual(["a", "b"], names) self.assertEqual(["a", "b"], names)
def test_child_git_repo_replaces_same_host(self): def test_child_git_repo_different_name_same_host_coexists(self):
replacement = {"url": "ssh://git@host-a/replacement.git", "key": {"provider": "static", "path": "/dev/null"}} # 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( m = _build(
base={"git-gate": {"repos": {"a": self._GIT_ENTRY_A}}}, base={"git-gate": {"repos": {"a": self._GIT_ENTRY_A}}},
child={ child={
"extends": "base", "extends": "base",
"git-gate": {"repos": {"a2": replacement}}, "git-gate": {"repos": {"a2": same_host_b}},
}, },
) )
entries = m.bottles["child"].git entries = m.bottles["child"].git
self.assertEqual(1, len(entries)) self.assertEqual(2, len(entries))
self.assertEqual("a2", entries[0].Name) names = {e.Name for e in entries}
self.assertEqual("replacement.git", entries[0].UpstreamPath) self.assertEqual({"a", "a2"}, names)
def test_child_omits_git_gate_inherits_full_list(self): def test_child_omits_git_gate_inherits_full_list(self):
m = _build( m = _build(
@@ -164,6 +166,77 @@ class TestExtendsGitMerge(unittest.TestCase):
) )
self.assertEqual((), m.bottles["child"].git) 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): def test_child_git_user_inherits_parent_repos(self):
m = _build( m = _build(
base={"git-gate": {"repos": {"a": self._GIT_ENTRY_A}}}, base={"git-gate": {"repos": {"a": self._GIT_ENTRY_A}}},