didericis/bot-bottle
1
0
Fork 0
Code Issues 7 Pull Requests 15 Actions Packages Projects Releases Wiki Activity

Better merge behavior for git-gate repos on extends #238

Merged
didericis merged 5 commits from git-gate-repo-field-merge-on-extends into main 2026-06-22 14:49:50 -04:00
Conversation 2 Commits 5 Files Changed 4
+155 -47
didericis-claude commented 2026-06-19 22:02:29 -04:00
Collaborator
Copy Link
Copy Source

Closes #237.

Summary

  • Adds _pre_merge_git_repos in manifest_extends.py: before parsing a child bottle, fills missing fields in same-named git-gate repo entries from the parent entry. Parent fields serve as defaults; child-declared fields win. This lets a child override only key: (or any subset of fields) without repeating url: and host_key:.
  • Adds _entry_to_raw helper to convert a ManifestGitEntry back to its YAML-equivalent raw dict (needed to reconstruct parent defaults for merging).
  • Changes _merge_git_remotes to key by Name instead of UpstreamHost. Name is the natural unique identity for a repo entry; host-keying incorrectly collapsed two repos with different names on the same host.
  • Three new test cases in TestExtendsGitMerge covering: field-merge of key: only, child URL override, and same-name-plus-new-name in one extend.
Closes #237. ## Summary - Adds `_pre_merge_git_repos` in `manifest_extends.py`: before parsing a child bottle, fills missing fields in same-named git-gate repo entries from the parent entry. Parent fields serve as defaults; child-declared fields win. This lets a child override only `key:` (or any subset of fields) without repeating `url:` and `host_key:`. - Adds `_entry_to_raw` helper to convert a `ManifestGitEntry` back to its YAML-equivalent raw dict (needed to reconstruct parent defaults for merging). - Changes `_merge_git_remotes` to key by `Name` instead of `UpstreamHost`. Name is the natural unique identity for a repo entry; host-keying incorrectly collapsed two repos with different names on the same host. - Three new test cases in `TestExtendsGitMerge` covering: field-merge of `key:` only, child URL override, and same-name-plus-new-name in one extend.
didericis-claude added 1 commit 2026-06-19 22:02:30 -04:00
feat(manifest-extends): field-merge same-name git-gate repos on extends
lint / lint (push) Successful in 1m34s
Details
test / unit (pull_request) Successful in 27s
Details
test / integration (pull_request) Successful in 15s
Details
4ed6b84863 
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
didericis reviewed 2026-06-19 22:20:46 -04:00
bot_bottle/manifest_extends.py Outdated
@@ -133,0 +154,4 @@
def _pre_merge_git_repos(
parent_git: "tuple[ManifestGitEntry, ...]",
child_raw: dict[str, object],
) -> dict[str, object]:
didericis commented 2026-06-19 22:20:46 -04:00
Owner
Copy Link
Copy Source

This is way too bespoke: I want a more generic deep merge for git-config repos that looks basically like this:

all_repo_names = set(child_config['git-gate']['repos'].keys()) + set(parent_config['git_gate']['repos'].keys())

merged_repo_configs = {}
for repo_name in all_repo_names:
    merged_repo_configs[repo_name] = {
        **parent_git_gate_config['git-gate']['repos'][repo_name],
        **child_git_gate_config['git-gate']['repos'][repo_name]
    }

Should use a safe accessor/account for possible values not being there, but this seems like way too many lines for a relatively simple merge

This is way too bespoke: I want a more generic deep merge for git-config repos that looks basically like this: ```python all_repo_names = set(child_config['git-gate']['repos'].keys()) + set(parent_config['git_gate']['repos'].keys()) merged_repo_configs = {} for repo_name in all_repo_names: merged_repo_configs[repo_name] = { **parent_git_gate_config['git-gate']['repos'][repo_name], **child_git_gate_config['git-gate']['repos'][repo_name] } ``` Should use a safe accessor/account for possible values not being there, but this seems like way too many lines for a relatively simple merge
didericis added 1 commit 2026-06-19 22:25:21 -04:00
refactor(manifest-extends): simplify git-gate repo merge to union + dict unpack
lint / lint (push) Failing after 1m31s
Details
test / unit (pull_request) Successful in 30s
Details
test / integration (pull_request) Successful in 17s
Details
ff7a52c1d2 
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.
didericis-claude reviewed 2026-06-19 22:46:36 -04:00
bot_bottle/manifest_extends.py Outdated
@@ -133,0 +154,4 @@
def _merge_git_repos_raw(
parent: "tuple[ManifestGitEntry, ...]",
didericis-claude commented 2026-06-19 22:46:36 -04:00
Author
Collaborator
Copy Link
Copy Source

Agreed, this is too bespoke. The clean version is a per-name shallow merge — git-gate.repos is already { <name>: <entry> } in the raw manifest, so:

merged_repos = {
    name: {**parent_repos.get(name, {}), **child_repos.get(name, {})}
    for name in {*parent_repos, *child_repos}
}

drops both _pre_merge_git_repos and _entry_to_raw, and makes the _merge_git_remotes host-keying change moot — name is the identity here, so the two-repos-on-one-host collision goes away without special-casing.

One wrinkle to flag: _merge_bottles currently receives the parent already parsed (and the parent may itself be a merge result), so there is no parent raw repos dict to merge against — that's the only reason _entry_to_raw existed. To keep the merge a pure dict overlay I'll thread the resolved repos dict through _resolve_one_bottle rather than reconstructing it from parsed entries. Reworking the PR to that shape and keeping the three TestExtendsGitMerge cases.

Agreed, this is too bespoke. The clean version is a per-name shallow merge — `git-gate.repos` is already `{ <name>: <entry> }` in the raw manifest, so: ```python merged_repos = { name: {**parent_repos.get(name, {}), **child_repos.get(name, {})} for name in {*parent_repos, *child_repos} } ``` drops both `_pre_merge_git_repos` and `_entry_to_raw`, and makes the `_merge_git_remotes` host-keying change moot — name is the identity here, so the two-repos-on-one-host collision goes away without special-casing. One wrinkle to flag: `_merge_bottles` currently receives the parent already *parsed* (and the parent may itself be a merge result), so there is no parent *raw* repos dict to merge against — that's the only reason `_entry_to_raw` existed. To keep the merge a pure dict overlay I'll thread the resolved repos dict through `_resolve_one_bottle` rather than reconstructing it from parsed entries. Reworking the PR to that shape and keeping the three `TestExtendsGitMerge` cases.
didericis added 1 commit 2026-06-19 22:53:32 -04:00
refactor(manifest-extends): thread resolved repos through recursion
lint / lint (push) Successful in 1m32s
Details
test / unit (pull_request) Successful in 28s
Details
test / integration (pull_request) Successful in 16s
Details
8f21f4df19 
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
didericis added 1 commit 2026-06-22 13:21:17 -04:00
fix(git-gate): skip host key-file check for gitea-provider repos
lint / lint (push) Successful in 1m39s
Details
test / unit (pull_request) Successful in 34s
Details
test / integration (pull_request) Successful in 18s
Details
9f97de115b 
_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>
didericis added 1 commit 2026-06-22 14:45:06 -04:00
refactor(backend): remove _validate_git_entries host key-file check
test / unit (pull_request) Successful in 37s
Details
test / integration (pull_request) Successful in 18s
Details
lint / lint (push) Successful in 1m39s
Details
test / unit (push) Successful in 37s
Details
test / integration (push) Successful in 18s
Details
Update Quality Badges / update-badges (push) Successful in 1m38s
Details
65faa40b9a 
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>
didericis merged commit 65faa40b9a into main 2026-06-22 14:49:50 -04:00
didericis deleted branch git-gate-repo-field-merge-on-extends 2026-06-22 14:49:51 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
didericis
didericis-claude
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#238
Delete Branch "git-gate-repo-field-merge-on-extends"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?