From b49dab6eed79fda81f34533fc52104db077b8f6f Mon Sep 17 00:00:00 2001 From: Petr Date: Thu, 7 May 2026 19:22:29 +0200 Subject: [PATCH] fix(0.30.4): close issue #267 -- chained sync git-branching bugs A/B/C/D/E Bug A: branch_link persisted Keboola branch IDs as str in .keboola/branch-mapping.json while every consumer compared them as int from ManifestBranch.id. Cross-type == was always False, so: - _find_branch_path fell back to manifest.branches[0].path ("main"), misrouting pulls into main/ instead of the linked branch dir. - _ensure_branch_registered re-registered the "unknown" branch on every pull, appending a duplicate branches[] entry with a mangled branch-- path. - The b.get("id") == branch_id API name lookup also failed, so the sanitized branch name from the API was never used and paths fell through to the numeric branch- fallback (the "side observation" in the original report). Fix is end-to-end int: branch_link writes int(branch_info["id"]) at five call-sites; BranchMappingEntry.keboola_id: int | None (was str | None); BranchMapping.from_dict silently coerces legacy str-id entries on load so existing user workspaces upgrade without manual editing. Bug B: _find_untracked_configs scoped the walk exclusively to branches with already-tracked configs, so manifest.configurations: [] silently dropped the documented ADDED -> push creates it flow on git-branching workspaces. Widened the scope to tracked U {default} U {resolved}; diff() now passes the resolved branch_id into the walker. Phantom-add protection for orphan dev-branch dirs is preserved. Bug C: branch_switched in pull() compared int existing_branch_ids against the str-poisoned branch_id, so the idempotency check was always True and every pull re-wrote every config. Auto-fixed by Bug A; regression test pins pull-pull-pull stability. Bug D: branch delete and branch merge now clean up matching entries from .keboola/branch-mapping.json in the nearest enclosing sync workspace. Pre-fix, deleting a Keboola dev branch left the local mapping pointing at a now-non-existent branch and every subsequent sync pull/push failed with 404. New helper cleanup_branch_id_from_mapping() in sync/branch_mapping.py walks upward from cwd, removes every entry whose keboola_id equals the deleted/merged branch ID, and is wired into delete_branch and get_merge_url. Bug E: _resolve_branch_id no longer raises ConfigError for the default git branch when branch-mapping.json is missing. Pre-fix, an accidentally lost mapping file blocked even sync pull on main with no recovery path (branch_link forbids linking the default branch). Fix returns None (production) for the default branch when the mapping is missing or has no entry. Non-default branches still raise so the user is told to link them. Tests: 7 new TestIssue267Regressions covering branch_link int persistence, repeated-pull manifest stability, pull routing to feature dir, walker untracked detection on empty configurations, walker phantom-add protection still holds, default-branch recovery on missing mapping, dev-branch error path on missing mapping. 6 new tests in test_sync_branch_mapping.py covering legacy str-id migration, find_sync_workspace upward search, cleanup_branch_id_from_mapping matching/no-op cases. Updated 8 existing assertions from keboola_id == "99999" to keboola_id == 99999 (the prior assertions locked Bug A in). E2E reproduced both bugs against a real Keboola project on v0.30.3, fixed in v0.30.4, and re-verified against the same project on the fix branch (logs preserved locally for the test design phase). After fix: branch_id is int in JSON output, branch_dir is the sanitized branch name, manifest.branches stable across N pulls, files_written goes to 0 after the first pull on unchanged configs, sync diff reports added: 1 for scaffolded local configs, branch delete returns mapping_cleanup with the unlinked git branches. --- .claude-plugin/marketplace.json | 2 +- plugins/kbagent/.claude-plugin/plugin.json | 2 +- pyproject.toml | 2 +- src/keboola_agent_cli/changelog.py | 9 + .../services/branch_service.py | 39 +- .../services/sync_service.py | 81 ++-- src/keboola_agent_cli/sync/branch_mapping.py | 63 ++- tests/test_sync_branch_mapping.py | 157 +++++++- tests/test_sync_service.py | 373 +++++++++++++++++- uv.lock | 2 +- 10 files changed, 666 insertions(+), 64 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index a8135ebe..12b7ce9b 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "kbagent", - "version": "0.30.3", + "version": "0.30.4", "source": "./plugins/kbagent", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "category": "development" diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index 1eecb863..ef3ea5d9 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.30.3", + "version": "0.30.4", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "author": { "name": "Keboola", diff --git a/pyproject.toml b/pyproject.toml index 1a770c47..8271eda2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-agent-cli" -version = "0.30.3" +version = "0.30.4" description = "AI-friendly CLI for managing Keboola projects" readme = "README.md" requires-python = ">=3.12" diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index e1c071de..fd25c81c 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -8,6 +8,15 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.30.4": [ + "Fix: `kbagent sync pull` against a linked dev branch now writes files under the linked branch's directory (`branch-/...` or its sanitized name), not under `main/`. Pre-fix, `branch_link` persisted Keboola branch IDs as **strings** in `.keboola/branch-mapping.json` (`kbc_branch_id = str(branch_info['id'])` at five call-sites in `services/sync_service.py`), but every comparison against the manifest read those IDs as the **int** they're typed as in `ManifestBranch.id: int` and on the Storage API. Cross-type `int == str` is always False in Python, so `_find_branch_path` fell back to the default branch (`manifest.branches[0].path == 'main'`) and `_ensure_branch_registered` re-registered the 'unknown' branch on every pull, appending a duplicate `branches[]` entry with a mangled `branch--` path until the manifest was hand-cleaned. The same comparison failed in `_ensure_branch_registered`'s `b.get('id') == branch_id` API-name lookup, so the branch's human-readable name from the API was never used and the path always fell through to the numeric `branch-` fallback (the 'side observation' from issue #267). Fix is end-to-end `int`: `branch_link` writes `int(branch_info['id'])`, `BranchMappingEntry.keboola_id: int | None` (was `str | None`), and `from_dict` silently coerces legacy string IDs on load so existing user workspaces upgrade without manual editing. Bug A from issue #267, reported externally on v0.27.0 and reproduced on v0.30.3.", + "Fix: `kbagent sync pull` no longer re-writes every previously-tracked config on every invocation in git-branching mode. The `branch_switched` guard at `services/sync_service.py:489-491` compared `existing_branch_ids[lookup_key]` (int from manifest) against the polluted str return of `_resolve_branch_id`; cross-type `!=` was always True, so the idempotency check was completely defeated and `files_written` ticked up on every pull even when nothing changed. The Bug A end-to-end int fix automatically restores correct behaviour here -- this is Bug C from issue #267, fixed transitively. Regression test pins `pull-pull-pull` against an unchanged remote and asserts manifest stability.", + "Fix: `kbagent sync diff` and `kbagent sync push --dry-run` now surface scaffolded local config directories on git-branching workspaces with empty `manifest.configurations[]`. Pre-fix, `_find_untracked_configs` (`services/sync_service.py:2612`) built its scope set exclusively from already-tracked configs (`active_branch_ids.add(cfg.branch_id)`); when configurations were empty, the scope set was empty, the walker `continue`d past every branch and returned `[]`, silently dropping the documented `ADDED -> push creates it` flow. Fix widens the scope to `tracked U {default} U {resolved}`: branches with tracked configs (today's protection against orphaned dirs), the default branch (push-to-main scaffold is legitimate), and the branch the caller resolved for this op (the linked feature branch the user is actively working on). `diff()` now passes `branch_id` into the walker so the resolved branch is in scope. Phantom-add protection for unrelated dev-branch dirs is preserved. Bug B from issue #267.", + "Fix: `kbagent branch delete` and `kbagent branch merge` now clean up matching entries from `.keboola/branch-mapping.json` in the nearest enclosing sync workspace. Pre-fix, deleting a Keboola dev branch left the local mapping pointing at a now-non-existent branch, and every subsequent `sync pull/push` from the linked git branch hit a 404 from the Storage API -- a non-recoverable state until the user manually ran `sync branch-unlink`. New helper `cleanup_branch_id_from_mapping()` in `sync/branch_mapping.py` walks upward from cwd to find the workspace, removes every entry whose `keboola_id` equals the deleted/merged branch ID, and is wired into `BranchService.delete_branch` and `BranchService.get_merge_url`. Both surface a `mapping_cleanup` field plus an additive message line listing the unlinked git branches. Bug D from issue #267.", + "Fix: `_resolve_branch_id` no longer raises `ConfigError` for the default git branch when `.keboola/branch-mapping.json` is missing. Pre-fix, an accidentally deleted (or `.gitignore`d) mapping file blocked even `sync pull` on the default branch with `Git branch 'main' is not linked to a Keboola branch` and there was no recovery path because `branch_link` explicitly forbids linking the default branch (`services/sync_service.py:1918`). Fix: when the mapping is missing or has no entry for the current branch AND the current branch is `manifest.git_branching.default_branch`, return `None` (production). Non-default branches with no mapping still raise `ConfigError` (intentionally narrow recovery: production is always reachable, dev branches still require explicit linking). Bug E from issue #267.", + 'Tests: 7 new regression tests under `TestIssue267Regressions` (covering branch_link int persistence, repeated-pull manifest stability, pull routing to feature dir, walker untracked-detection on empty configurations, walker phantom-add protection still holds, default-branch recovery on missing mapping, dev-branch error path on missing mapping) plus 6 new tests in `tests/test_sync_branch_mapping.py` covering legacy str-id migration, `find_sync_workspace` upward search, and `cleanup_branch_id_from_mapping` cases (matching id removal, unmatched no-op, no-workspace no-op). Existing tests in `TestBranchLink`, `TestBranchUnlink`, `TestBranchStatus`, and `test_sync_branch_mapping.py` updated from `assert keboola_id == "99999"` (the assertion that locked Bug A in) to `assert keboola_id == 99999`.', + "Refactor: `_find_untracked_configs(project_root, manifest)` is now `_find_untracked_configs(project_root, manifest, resolved_branch_id=None)`. The scope-widening param defaults to `None` (keeps `status()` callsite at `services/sync_service.py:791` semantics-compatible). `diff()` callsite at line 905 now passes the resolved branch ID so the walker covers the linked feature branch dir. No behaviour change for non-git-branching workspaces.", + ], "0.30.3": [ "Fix: `_perform_mcp_update` for `uvx`-cache installs now promotes to `uv tool install --upgrade keboola-mcp-server` instead of running the broken `uvx --refresh --from --version` chain. The trailing `--version` arg was rejected by the upstream MCP binary (no such flag), so the upgrade subprocess always exited non-zero and the user-facing banner reported failure even when the cache refresh itself worked. Promoting to `uv tool install --upgrade` does the equivalent refresh AND moves the binary to PATH so subsequent runs use the faster `uv_tool` detection path. Bug B fix from issue #263.", "Fix: `_maybe_update_mcp` now skips the upgrade attempt when the local-version probe returns `None`. Pre-fix, probe-`None` left `up_to_date == None` (not `True`), the short-circuit was bypassed, and the function fell through to a broken upgrade subprocess every TTL window. The user saw an `Updating ... vunknown -> v1.59.1` banner once per kbagent invocation. Post-fix, probe-`None` opts out of the upgrade for this TTL window; the next fresh-cache pass will retry detection. Cache TTL still ticks. Bug C fix from issue #263.", diff --git a/src/keboola_agent_cli/services/branch_service.py b/src/keboola_agent_cli/services/branch_service.py index 51aac872..df602a6c 100644 --- a/src/keboola_agent_cli/services/branch_service.py +++ b/src/keboola_agent_cli/services/branch_service.py @@ -255,6 +255,11 @@ def reset_branch(self, alias: str) -> dict[str, Any]: def delete_branch(self, alias: str, branch_id: int) -> dict[str, Any]: """Delete a development branch via API. Auto-resets if it was active. + Also cleans up any matching entry in the nearest enclosing sync + workspace's ``branch-mapping.json`` (issue #267, Bug D), so the + user is not left with a stale mapping pointing at a deleted + branch. + Args: alias: Project alias. branch_id: Branch ID to delete. @@ -266,6 +271,8 @@ def delete_branch(self, alias: str, branch_id: int) -> dict[str, Any]: ConfigError: If the project alias is not found. KeboolaApiError: If the API call fails. """ + from ..sync.branch_mapping import cleanup_branch_id_from_mapping + projects = self.resolve_projects([alias]) project = projects[alias] @@ -280,15 +287,24 @@ def delete_branch(self, alias: str, branch_id: int) -> dict[str, Any]: if was_active: self._config_store.set_project_branch(alias, None) - return { + cleanup = cleanup_branch_id_from_mapping(branch_id) + + message_parts = [f"Branch ID {branch_id} deleted from project '{alias}'."] + if was_active: + message_parts.append("Active branch reset to main.") + if cleanup: + unlinked = ", ".join(cleanup["git_branches_unlinked"]) + message_parts.append(f"Unlinked git branch(es): {unlinked}.") + + result: dict[str, Any] = { "project_alias": alias, "branch_id": branch_id, "was_active": was_active, - "message": ( - f"Branch ID {branch_id} deleted from project '{alias}'." - + (" Active branch reset to main." if was_active else "") - ), + "message": " ".join(message_parts), } + if cleanup: + result["mapping_cleanup"] = cleanup + return result def get_merge_url(self, alias: str, branch_id: int | None = None) -> dict[str, Any]: """Generate KBC UI merge URL for a development branch. @@ -332,7 +348,13 @@ def get_merge_url(self, alias: str, branch_id: int | None = None) -> dict[str, A # Reset active branch to main after generating merge URL self._config_store.set_project_branch(alias, None) - return { + # Best-effort cleanup of any matching sync-workspace mapping so the + # user is not left referencing a soon-to-be-merged branch (Bug D). + from ..sync.branch_mapping import cleanup_branch_id_from_mapping + + cleanup = cleanup_branch_id_from_mapping(effective_branch_id) + + result: dict[str, Any] = { "project_alias": alias, "branch_id": effective_branch_id, "url": merge_url, @@ -341,6 +363,11 @@ def get_merge_url(self, alias: str, branch_id: int | None = None) -> dict[str, A f"in project '{alias}'. Active branch has been reset to main." ), } + if cleanup: + unlinked = ", ".join(cleanup["git_branches_unlinked"]) + result["message"] += f" Unlinked git branch(es): {unlinked}." + result["mapping_cleanup"] = cleanup + return result # ── Branch metadata ──────────────────────────────────────────────── diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index 4312ed72..02cc4124 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -902,7 +902,7 @@ def diff( ) # Also add untracked local configs (new files) - for added_cfg in self._find_untracked_configs(project_root, manifest): + for added_cfg in self._find_untracked_configs(project_root, manifest, branch_id): branch_path = self._find_branch_path(manifest, branch_id) config_dir = project_root / branch_path / added_cfg["path"] local_data = self._read_config_file(config_dir) @@ -1954,7 +1954,7 @@ def branch_link( ) if branch_info is None: raise ConfigError(f"Keboola branch {branch_id} not found.") - kbc_branch_id = str(branch_info["id"]) + kbc_branch_id = int(branch_info["id"]) kbc_branch_name = branch_info.get("name", "") elif branch_name: # Search by name or create @@ -1964,11 +1964,11 @@ def branch_link( None, ) if branch_info: - kbc_branch_id = str(branch_info["id"]) + kbc_branch_id = int(branch_info["id"]) kbc_branch_name = branch_info.get("name", "") else: result = client.create_dev_branch(name=branch_name) - kbc_branch_id = str(result["id"]) + kbc_branch_id = int(result["id"]) kbc_branch_name = branch_name else: # Default: use git branch name to search/create @@ -1978,11 +1978,11 @@ def branch_link( None, ) if branch_info: - kbc_branch_id = str(branch_info["id"]) + kbc_branch_id = int(branch_info["id"]) kbc_branch_name = branch_info.get("name", "") else: result = client.create_dev_branch(name=git_branch) - kbc_branch_id = str(result["id"]) + kbc_branch_id = int(result["id"]) kbc_branch_name = git_branch mapping.set(git_branch, kbc_branch_id, kbc_branch_name) @@ -2095,7 +2095,12 @@ def _resolve_branch_id( 3. First branch in manifest (production fallback) Raises ``ConfigError`` if git-branching is enabled but the current - branch is not linked (prevents accidental production writes). + branch is not the default and is not linked. + + The default git branch always resolves to ``None`` (production), even + if ``branch-mapping.json`` is missing or has no entry for it. This + guarantees there is always a recovery path when the mapping file is + lost (issue #267, Bug E). """ from ..sync.branch_mapping import load_branch_mapping from ..sync.git_utils import get_current_branch @@ -2103,22 +2108,34 @@ def _resolve_branch_id( if manifest.git_branching.enabled: git_branch = get_current_branch(project_root) if git_branch: + default_branch = manifest.git_branching.default_branch + is_default = git_branch == default_branch try: mapping = load_branch_mapping(project_root) - entry = mapping.get(git_branch) - if entry is not None: - # entry.keboola_id is None for production (default branch) - return entry.keboola_id except FileNotFoundError: - pass - # Branch not linked -- block operation + # Mapping missing -- auto-recover for the default branch + # so the user is never locked out of production. + if is_default: + return None + raise ConfigError( + f"Git branch '{git_branch}' is not linked to a Keboola " + f"branch (branch-mapping.json missing). " + f"Run 'kbagent sync branch-link --project ALIAS' first." + ) from None + entry = mapping.get(git_branch) + if entry is not None: + # entry.keboola_id is None for production (default branch) + return entry.keboola_id + # No entry for current branch -- default branch is always production + if is_default: + return None raise ConfigError( f"Git branch '{git_branch}' is not linked to a Keboola branch. " f"Run 'kbagent sync branch-link --project ALIAS' first." ) # Non git-branching: use active_branch_id or manifest fallback - branch_id = project.active_branch_id + branch_id = project.active_branch_id if project is not None else None if not branch_id and manifest.branches: branch_id = manifest.branches[0].id return branch_id @@ -2610,26 +2627,44 @@ def _find_branch_path(self, manifest: Manifest, branch_id: int | None) -> str: return manifest.branches[0].path if manifest.branches else "main" def _find_untracked_configs( - self, project_root: Path, manifest: Manifest + self, + project_root: Path, + manifest: Manifest, + resolved_branch_id: int | None = None, ) -> list[dict[str, str]]: """Scan for _config.yml files that are not tracked in the manifest. - Only scans branch directories that have at least one tracked - configuration. This prevents phantom "added" configs from - inactive/old branch directories left over from a previous pull. + Scans branch directories that the user is actively working with: + branches that already have tracked configs, the default branch + (production), and the branch the caller resolved for the current + operation (when provided). This supports the documented + "scaffold locally then push" workflow on git-branching workspaces + with empty ``manifest.configurations`` (issue #267, Bug B). + + Branches outside this scope are skipped to avoid phantom "added" + configs from orphaned dev-branch directories left over from + previous work. """ tracked_paths: set[str] = set() - active_branch_ids: set[int] = set() + in_scope_branch_ids: set[int] = set() for cfg in manifest.configurations: branch_path = self._find_branch_path(manifest, cfg.branch_id) tracked_paths.add(str(project_root / branch_path / cfg.path)) - active_branch_ids.add(cfg.branch_id) + in_scope_branch_ids.add(cfg.branch_id) + + # Default branch is always in scope -- pushing a brand-new config + # against production with empty configurations[] is a legitimate flow. + if manifest.branches: + in_scope_branch_ids.add(manifest.branches[0].id) + + # The branch the caller resolved is in scope (linked feature branch + # the user explicitly switched to via git checkout + branch-link). + if resolved_branch_id is not None: + in_scope_branch_ids.add(resolved_branch_id) added: list[dict[str, str]] = [] for branch in manifest.branches: - # Only scan branches that have tracked configs — skip inactive - # branch directories to avoid phantom "added" configs. - if branch.id not in active_branch_ids: + if branch.id not in in_scope_branch_ids: continue branch_dir = project_root / branch.path if not branch_dir.exists(): diff --git a/src/keboola_agent_cli/sync/branch_mapping.py b/src/keboola_agent_cli/sync/branch_mapping.py index 89afac21..a3d0e2b3 100644 --- a/src/keboola_agent_cli/sync/branch_mapping.py +++ b/src/keboola_agent_cli/sync/branch_mapping.py @@ -13,10 +13,22 @@ from ..constants import BRANCH_MAPPING_FILENAME, KEBOOLA_DIR_NAME +def _coerce_keboola_id(raw: Any) -> int | None: + """Coerce a raw ``id`` field from JSON to ``int | None``. + + Older kbagent versions (<= 0.30.3) wrote branch IDs as strings (e.g. + ``"99999"``) due to issue #267. ``None`` means production. Empty + string is also treated as production for legacy tolerance. + """ + if raw is None or raw == "": + return None + return int(raw) + + class BranchMappingEntry: """A single git branch -> Keboola branch mapping.""" - def __init__(self, keboola_id: str | None, name: str): + def __init__(self, keboola_id: int | None, name: str): self.keboola_id = keboola_id # None = production self.name = name @@ -37,7 +49,7 @@ def __init__(self) -> None: def get(self, git_branch: str) -> BranchMappingEntry | None: return self.mappings.get(git_branch) - def set(self, git_branch: str, keboola_id: str | None, name: str) -> None: + def set(self, git_branch: str, keboola_id: int | None, name: str) -> None: self.mappings[git_branch] = BranchMappingEntry(keboola_id, name) def remove(self, git_branch: str) -> bool: @@ -58,7 +70,7 @@ def from_dict(cls, data: dict[str, Any]) -> BranchMapping: mapping.version = data.get("version", 1) for git_branch, entry in data.get("mappings", {}).items(): mapping.mappings[git_branch] = BranchMappingEntry( - keboola_id=entry.get("id"), + keboola_id=_coerce_keboola_id(entry.get("id")), name=entry.get("name", ""), ) return mapping @@ -81,3 +93,48 @@ def save_branch_mapping(project_root: Path, mapping: BranchMapping) -> None: json.dumps(mapping.to_dict(), indent=4, ensure_ascii=False) + "\n", encoding="utf-8", ) + + +def find_sync_workspace(start: Path | None = None) -> Path | None: + """Locate the nearest enclosing sync workspace. + + Walks up from *start* (or the current working directory) and returns + the first directory that contains a ``.keboola/branch-mapping.json`` + file, or ``None`` if none is found before the filesystem root. + """ + cursor = (start or Path.cwd()).resolve() + for candidate in [cursor, *cursor.parents]: + if (candidate / KEBOOLA_DIR_NAME / BRANCH_MAPPING_FILENAME).exists(): + return candidate + return None + + +def cleanup_branch_id_from_mapping(branch_id: int) -> dict[str, Any] | None: + """Remove every git-branch entry that maps to *branch_id* from the + nearest enclosing sync workspace, if one exists. + + Designed to be a best-effort cleanup hook for ``branch delete`` and + ``branch merge``: locates ``.keboola/branch-mapping.json`` via + :func:`find_sync_workspace`, removes any entries whose ``keboola_id`` + equals *branch_id*, and persists the change. Returns a dict + describing what was unlinked, or ``None`` if no workspace was found + or no entry referenced the branch (no-op). + """ + project_root = find_sync_workspace() + if project_root is None: + return None + try: + mapping = load_branch_mapping(project_root) + except (FileNotFoundError, ValueError): + return None + + removed: list[str] = [] + for git_branch, entry in list(mapping.mappings.items()): + if entry.keboola_id == branch_id: + mapping.remove(git_branch) + removed.append(git_branch) + + if not removed: + return None + save_branch_mapping(project_root, mapping) + return {"project_root": str(project_root), "git_branches_unlinked": removed} diff --git a/tests/test_sync_branch_mapping.py b/tests/test_sync_branch_mapping.py index fb494734..0b30ae3e 100644 --- a/tests/test_sync_branch_mapping.py +++ b/tests/test_sync_branch_mapping.py @@ -1,7 +1,9 @@ """Tests for BranchMapping model and I/O (branch_mapping.py). Covers the BranchMappingEntry and BranchMapping classes, as well -as load/save filesystem round-trips. +as load/save filesystem round-trips. After issue #267, ``keboola_id`` +is ``int | None``; legacy str-typed values written by older versions +are coerced on load. """ import json @@ -13,6 +15,8 @@ from keboola_agent_cli.sync.branch_mapping import ( BranchMapping, BranchMappingEntry, + cleanup_branch_id_from_mapping, + find_sync_workspace, load_branch_mapping, save_branch_mapping, ) @@ -30,15 +34,15 @@ def test_branch_mapping_entry_production(self) -> None: def test_branch_mapping_entry_dev_branch(self) -> None: """Non-None keboola_id indicates development branch.""" - entry = BranchMappingEntry(keboola_id="972851", name="feature/auth") + entry = BranchMappingEntry(keboola_id=972851, name="feature/auth") assert entry.is_production() is False - assert entry.keboola_id == "972851" + assert entry.keboola_id == 972851 assert entry.name == "feature/auth" def test_branch_mapping_entry_to_dict(self) -> None: """to_dict returns the correct JSON-ready structure.""" - entry = BranchMappingEntry(keboola_id="12345", name="my-branch") - assert entry.to_dict() == {"id": "12345", "name": "my-branch"} + entry = BranchMappingEntry(keboola_id=12345, name="my-branch") + assert entry.to_dict() == {"id": 12345, "name": "my-branch"} def test_branch_mapping_entry_production_to_dict(self) -> None: """Production entry serializes id as None.""" @@ -53,7 +57,7 @@ def test_branch_mapping_set_get(self) -> None: """set and get work correctly.""" mapping = BranchMapping() mapping.set("main", None, "Main") - mapping.set("feature/auth", "972851", "feature/auth") + mapping.set("feature/auth", 972851, "feature/auth") main_entry = mapping.get("main") assert main_entry is not None @@ -62,7 +66,7 @@ def test_branch_mapping_set_get(self) -> None: feature_entry = mapping.get("feature/auth") assert feature_entry is not None - assert feature_entry.keboola_id == "972851" + assert feature_entry.keboola_id == 972851 assert feature_entry.name == "feature/auth" def test_branch_mapping_get_nonexistent(self) -> None: @@ -73,7 +77,7 @@ def test_branch_mapping_get_nonexistent(self) -> None: def test_branch_mapping_remove(self) -> None: """remove deletes an existing mapping and returns True.""" mapping = BranchMapping() - mapping.set("feature/auth", "972851", "feature/auth") + mapping.set("feature/auth", 972851, "feature/auth") assert mapping.remove("feature/auth") is True assert mapping.get("feature/auth") is None @@ -87,8 +91,8 @@ def test_branch_mapping_round_trip(self) -> None: """to_dict/from_dict round-trip preserves data.""" mapping = BranchMapping() mapping.set("main", None, "Main") - mapping.set("feature/auth", "972851", "feature/auth") - mapping.set("bugfix/123", "88888", "bugfix/123") + mapping.set("feature/auth", 972851, "feature/auth") + mapping.set("bugfix/123", 88888, "bugfix/123") data = mapping.to_dict() restored = BranchMapping.from_dict(data) @@ -103,23 +107,23 @@ def test_branch_mapping_round_trip(self) -> None: auth_entry = restored.get("feature/auth") assert auth_entry is not None - assert auth_entry.keboola_id == "972851" + assert auth_entry.keboola_id == 972851 assert auth_entry.name == "feature/auth" bugfix_entry = restored.get("bugfix/123") assert bugfix_entry is not None - assert bugfix_entry.keboola_id == "88888" + assert bugfix_entry.keboola_id == 88888 def test_branch_mapping_to_dict_format(self) -> None: """to_dict produces the Go CLI compatible format.""" mapping = BranchMapping() mapping.set("main", None, "Main") - mapping.set("feature/auth", "972851", "feature/auth") + mapping.set("feature/auth", 972851, "feature/auth") data = mapping.to_dict() assert data["version"] == 1 assert data["mappings"]["main"] == {"id": None, "name": "Main"} - assert data["mappings"]["feature/auth"] == {"id": "972851", "name": "feature/auth"} + assert data["mappings"]["feature/auth"] == {"id": 972851, "name": "feature/auth"} def test_branch_mapping_from_dict_empty(self) -> None: """from_dict handles empty mappings.""" @@ -130,11 +134,34 @@ def test_branch_mapping_from_dict_empty(self) -> None: def test_branch_mapping_from_dict_defaults(self) -> None: """from_dict uses defaults when fields are missing.""" - data = {} + data: dict = {} mapping = BranchMapping.from_dict(data) assert mapping.version == 1 assert len(mapping.mappings) == 0 + def test_branch_mapping_from_dict_legacy_string_id(self) -> None: + """Issue #267: legacy ``branch-mapping.json`` with str-typed ``id`` is + silently migrated to int on load. This guarantees existing users do + not need to manually edit their workspace after upgrading.""" + data = { + "version": 1, + "mappings": { + "main": {"id": None, "name": "Main"}, + "feature/auth": {"id": "972851", "name": "feature/auth"}, + "bugfix/123": {"id": "88888", "name": "bugfix/123"}, + }, + } + mapping = BranchMapping.from_dict(data) + assert mapping.get("main").keboola_id is None + assert mapping.get("feature/auth").keboola_id == 972851 + assert mapping.get("bugfix/123").keboola_id == 88888 + + def test_branch_mapping_from_dict_empty_string_id(self) -> None: + """Empty string id (rare legacy shape) is treated as production (None).""" + data = {"version": 1, "mappings": {"main": {"id": "", "name": "Main"}}} + mapping = BranchMapping.from_dict(data) + assert mapping.get("main").keboola_id is None + class TestBranchMappingIO: """Tests for load/save filesystem operations.""" @@ -143,7 +170,7 @@ def test_load_save_branch_mapping(self, tmp_path: Path) -> None: """Filesystem round-trip: save then load preserves data.""" mapping = BranchMapping() mapping.set("main", None, "Main") - mapping.set("feature/auth", "972851", "feature/auth") + mapping.set("feature/auth", 972851, "feature/auth") save_branch_mapping(tmp_path, mapping) @@ -151,11 +178,11 @@ def test_load_save_branch_mapping(self, tmp_path: Path) -> None: path = tmp_path / KEBOOLA_DIR_NAME / BRANCH_MAPPING_FILENAME assert path.exists() - # Verify raw JSON content + # Verify raw JSON content -- ids written as int, not str (issue #267) raw = json.loads(path.read_text(encoding="utf-8")) assert raw["version"] == 1 assert raw["mappings"]["main"]["id"] is None - assert raw["mappings"]["feature/auth"]["id"] == "972851" + assert raw["mappings"]["feature/auth"]["id"] == 972851 # Load and verify loaded = load_branch_mapping(tmp_path) @@ -168,7 +195,7 @@ def test_load_save_branch_mapping(self, tmp_path: Path) -> None: auth_entry = loaded.get("feature/auth") assert auth_entry is not None - assert auth_entry.keboola_id == "972851" + assert auth_entry.keboola_id == 972851 def test_load_branch_mapping_not_found(self, tmp_path: Path) -> None: """load_branch_mapping raises FileNotFoundError when file is missing.""" @@ -196,8 +223,98 @@ def test_save_overwrites_existing(self, tmp_path: Path) -> None: mapping2 = BranchMapping() mapping2.set("main", None, "Main") - mapping2.set("develop", "99999", "develop") + mapping2.set("develop", 99999, "develop") save_branch_mapping(tmp_path, mapping2) loaded = load_branch_mapping(tmp_path) assert len(loaded.mappings) == 2 + + def test_load_silently_migrates_legacy_string_ids_on_disk(self, tmp_path: Path) -> None: + """A ``branch-mapping.json`` written by older kbagent versions + with string-typed ``id`` loads and produces int-typed + ``keboola_id`` (issue #267).""" + keboola_dir = tmp_path / KEBOOLA_DIR_NAME + keboola_dir.mkdir() + legacy_path = keboola_dir / BRANCH_MAPPING_FILENAME + legacy_path.write_text( + json.dumps( + { + "version": 1, + "mappings": { + "main": {"id": None, "name": "Main"}, + "feature/auth": {"id": "972851", "name": "feature/auth"}, + }, + } + ) + ) + loaded = load_branch_mapping(tmp_path) + feature = loaded.get("feature/auth") + assert feature is not None + assert feature.keboola_id == 972851 # not "972851" + + +class TestSyncWorkspaceHelpers: + """Tests for find_sync_workspace and cleanup_branch_id_from_mapping (issue #267, Bug D).""" + + def test_find_sync_workspace_locates_nearest_ancestor( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """``find_sync_workspace`` walks upward and finds the closest enclosing + ``.keboola/branch-mapping.json``.""" + workspace = tmp_path / "my-workspace" + nested = workspace / "src" / "feature" + nested.mkdir(parents=True) + save_branch_mapping(workspace, BranchMapping()) + + monkeypatch.chdir(nested) + found = find_sync_workspace() + assert found is not None + assert found.resolve() == workspace.resolve() + + def test_find_sync_workspace_returns_none_when_outside( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns ``None`` when no ancestor contains a sync workspace.""" + unrelated = tmp_path / "unrelated" + unrelated.mkdir() + monkeypatch.chdir(unrelated) + assert find_sync_workspace() is None + + def test_cleanup_removes_only_matching_branch_id( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """``cleanup_branch_id_from_mapping`` removes every git branch that + maps to the given branch ID and leaves others intact.""" + mapping = BranchMapping() + mapping.set("main", None, "Main") + mapping.set("feature/a", 11111, "branch-a") + mapping.set("feature/b", 22222, "branch-b") + save_branch_mapping(tmp_path, mapping) + + monkeypatch.chdir(tmp_path) + result = cleanup_branch_id_from_mapping(11111) + assert result is not None + assert result["git_branches_unlinked"] == ["feature/a"] + + # Reload and confirm the change persisted + reloaded = load_branch_mapping(tmp_path) + assert reloaded.get("feature/a") is None + assert reloaded.get("feature/b") is not None + assert reloaded.get("main") is not None + + def test_cleanup_returns_none_when_branch_not_referenced( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns ``None`` when no entry references the branch (no-op).""" + mapping = BranchMapping() + mapping.set("main", None, "Main") + save_branch_mapping(tmp_path, mapping) + monkeypatch.chdir(tmp_path) + assert cleanup_branch_id_from_mapping(99999) is None + + def test_cleanup_returns_none_when_no_workspace( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Returns ``None`` when no enclosing sync workspace is found.""" + monkeypatch.chdir(tmp_path) + assert cleanup_branch_id_from_mapping(99999) is None diff --git a/tests/test_sync_service.py b/tests/test_sync_service.py index 98007886..a106f609 100644 --- a/tests/test_sync_service.py +++ b/tests/test_sync_service.py @@ -1600,7 +1600,7 @@ def test_branch_link_creates_branch(self, tmp_config_dir: Path, tmp_path: Path) assert result["status"] == "linked" assert result["git_branch"] == "feature/auth" - assert result["keboola_branch_id"] == "99999" + assert result["keboola_branch_id"] == 99999 assert result["keboola_branch_name"] == "feature/auth" link_client.create_dev_branch.assert_called_once_with(name="feature/auth") @@ -1610,7 +1610,7 @@ def test_branch_link_creates_branch(self, tmp_config_dir: Path, tmp_path: Path) mapping = load_branch_mapping(project_root) entry = mapping.get("feature/auth") assert entry is not None - assert entry.keboola_id == "99999" + assert entry.keboola_id == 99999 def test_branch_link_finds_existing_branch(self, tmp_config_dir: Path, tmp_path: Path) -> None: """branch_link links to an existing Keboola branch that matches the name.""" @@ -1639,7 +1639,7 @@ def test_branch_link_finds_existing_branch(self, tmp_config_dir: Path, tmp_path: assert result["status"] == "linked" assert result["git_branch"] == "feature-x" - assert result["keboola_branch_id"] == "99999" + assert result["keboola_branch_id"] == 99999 # Should not have created a new branch link_client.create_dev_branch.assert_not_called() @@ -1719,7 +1719,7 @@ def test_branch_link_already_linked(self, tmp_config_dir: Path, tmp_path: Path) assert result["status"] == "already_linked" assert result["git_branch"] == "feature-x" - assert result["keboola_branch_id"] == "99999" + assert result["keboola_branch_id"] == 99999 def test_branch_link_with_branch_id(self, tmp_config_dir: Path, tmp_path: Path) -> None: """branch_link with --branch-id links to a specific existing branch.""" @@ -1748,7 +1748,7 @@ def test_branch_link_with_branch_id(self, tmp_config_dir: Path, tmp_path: Path) ) assert result["status"] == "linked" - assert result["keboola_branch_id"] == "99999" + assert result["keboola_branch_id"] == 99999 assert result["keboola_branch_name"] == "feature-x" def test_branch_link_with_branch_name_creates( @@ -1781,7 +1781,7 @@ def test_branch_link_with_branch_name_creates( ) assert result["status"] == "linked" - assert result["keboola_branch_id"] == "77777" + assert result["keboola_branch_id"] == 77777 assert result["keboola_branch_name"] == "custom-name" link_client.create_dev_branch.assert_called_once_with(name="custom-name") @@ -1858,7 +1858,7 @@ def test_branch_unlink_success(self, tmp_config_dir: Path, tmp_path: Path) -> No assert result["status"] == "unlinked" assert result["git_branch"] == "feature-x" - assert result["keboola_branch_id"] == "99999" + assert result["keboola_branch_id"] == 99999 assert result["keboola_branch_name"] == "feature-x" # Verify mapping was removed from disk @@ -1976,7 +1976,7 @@ def test_branch_status_linked(self, tmp_config_dir: Path, tmp_path: Path) -> Non assert result["git_branching"] is True assert result["git_branch"] == "feature-x" assert result["linked"] is True - assert result["keboola_branch_id"] == "99999" + assert result["keboola_branch_id"] == 99999 assert result["keboola_branch_name"] == "feature-x" assert result["is_production"] is False @@ -2375,3 +2375,360 @@ def test_adopt_existing_idempotent(self, tmp_config_dir: Path, tmp_path: Path) - assert result2["status"] == "adopted" # Both calls return the same project data assert result1["project_id"] == result2["project_id"] + + +# =================================================================== +# Issue #267 regression tests +# =================================================================== + + +class TestIssue267Regressions: + """Regression coverage for the chained sync git-branching bugs (issue #267). + + Each test would have failed against ``main`` before the fix: + + * Bug A — branch_id type confusion (str in branch-mapping.json, int in + manifest) caused ``branch.id == branch_id`` cross-type compares to + always be False, misrouting pulls to ``main/`` and inflating + ``manifest.branches[]`` on every call. + * Bug B — ``_find_untracked_configs`` walker scope was scoped to + branches with already-tracked configs, blocking the documented + "scaffold locally then push" flow when ``configurations: []``. + * Bug E — ``_resolve_branch_id`` raised ``ConfigError`` for the + default git branch when ``branch-mapping.json`` was missing, + leaving users with no recovery path. + """ + + def _init_git_branching_project( + self, + tmp_config_dir: Path, + project_root: Path, + ) -> ConfigStore: + init_client = _make_sync_mock_client( + verify_token_response=SAMPLE_VERIFY_TOKEN, + branches_response=SAMPLE_BRANCHES, + ) + store = setup_single_project(tmp_config_dir) + init_svc = SyncService( + config_store=store, + client_factory=lambda url, token: init_client, + ) + with ( + patch( + "keboola_agent_cli.services.sync_service.is_git_repo", + return_value=True, + ), + patch( + "keboola_agent_cli.services.sync_service.get_default_branch", + return_value="main", + ), + ): + init_svc.init_sync( + alias="prod", + project_root=project_root, + git_branching=True, + ) + return store + + # ------------------------------------------------------------------ + # Bug A + # ------------------------------------------------------------------ + + def test_branch_link_persists_keboola_id_as_int( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """branch-mapping.json on disk stores ``id`` as int, never str.""" + project_root = tmp_path / "project" + project_root.mkdir() + store = self._init_git_branching_project(tmp_config_dir, project_root) + + link_client = _make_sync_mock_client(branches_response=SAMPLE_BRANCHES_WITH_DEV) + svc = SyncService( + config_store=store, + client_factory=lambda url, token: link_client, + ) + + with patch( + "keboola_agent_cli.sync.git_utils.get_current_branch", + return_value="feature-x", + ): + svc.branch_link( + alias="prod", + project_root=project_root, + branch_id=99999, + ) + + # Read the file directly: persisted JSON must have id as a JSON number. + raw = json.loads((project_root / KEBOOLA_DIR_NAME / BRANCH_MAPPING_FILENAME).read_text()) + assert raw["mappings"]["feature-x"]["id"] == 99999 + assert isinstance(raw["mappings"]["feature-x"]["id"], int) + + def test_repeated_pulls_do_not_grow_manifest_branches( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Running ``sync pull`` N times against a linked dev branch must not + append duplicate entries to ``manifest.branches`` (Bug A symptom).""" + project_root = tmp_path / "project" + project_root.mkdir() + store = self._init_git_branching_project(tmp_config_dir, project_root) + + # Link feature-x -> 99999 + link_client = _make_sync_mock_client(branches_response=SAMPLE_BRANCHES_WITH_DEV) + link_svc = SyncService( + config_store=store, + client_factory=lambda url, token: link_client, + ) + with patch( + "keboola_agent_cli.sync.git_utils.get_current_branch", + return_value="feature-x", + ): + link_svc.branch_link( + alias="prod", + project_root=project_root, + branch_id=99999, + ) + + # Pull three times against the same linked branch + pull_client = _make_sync_mock_client( + components_response=[], + branches_response=SAMPLE_BRANCHES_WITH_DEV, + ) + pull_client.list_buckets_with_metadata.return_value = [] + pull_client.list_tables_with_metadata.return_value = [] + pull_client.list_jobs_grouped.return_value = [] + pull_svc = SyncService( + config_store=store, + client_factory=lambda url, token: pull_client, + ) + for _ in range(3): + pull_svc.pull( + alias="prod", + project_root=project_root, + no_storage=True, + no_jobs=True, + ) + + manifest = load_manifest(project_root) + # Default branch + linked dev branch -- nothing more. + assert len(manifest.branches) == 2 + ids = sorted(b.id for b in manifest.branches) + assert ids == [12345, 99999] + # The dev branch path is the API-provided name (sanitized), not a + # numeric fallback. Bug A's type confusion previously prevented the + # name lookup from succeeding, leaving us with ``branch-99999``. + paths = sorted(b.path for b in manifest.branches) + assert paths == ["feature-x", "main"] + + def test_pull_response_routes_to_linked_branch_dir( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """``sync pull`` against a linked dev branch reports ``branch_dir`` as + the linked branch path, not ``main`` (Bug A misroute).""" + project_root = tmp_path / "project" + project_root.mkdir() + store = self._init_git_branching_project(tmp_config_dir, project_root) + + link_client = _make_sync_mock_client(branches_response=SAMPLE_BRANCHES_WITH_DEV) + link_svc = SyncService( + config_store=store, + client_factory=lambda url, token: link_client, + ) + with patch( + "keboola_agent_cli.sync.git_utils.get_current_branch", + return_value="feature-x", + ): + link_svc.branch_link( + alias="prod", + project_root=project_root, + branch_id=99999, + ) + + pull_client = _make_sync_mock_client( + components_response=[], + branches_response=SAMPLE_BRANCHES_WITH_DEV, + ) + pull_client.list_buckets_with_metadata.return_value = [] + pull_client.list_tables_with_metadata.return_value = [] + pull_client.list_jobs_grouped.return_value = [] + pull_svc = SyncService( + config_store=store, + client_factory=lambda url, token: pull_client, + ) + result = pull_svc.pull( + alias="prod", + project_root=project_root, + no_storage=True, + no_jobs=True, + ) + + # branch_id in the response is an int (issue #267) and branch_dir + # is the linked branch's path, never "main". + assert result["branch_id"] == 99999 + assert result["branch_dir"] != "main" + + # ------------------------------------------------------------------ + # Bug B + # ------------------------------------------------------------------ + + def test_walker_finds_untracked_with_empty_configurations(self, tmp_path: Path) -> None: + """``_find_untracked_configs`` surfaces a scaffold under a non-default + branch even when ``manifest.configurations: []``, given that the + branch is the resolved one for this op (Bug B fix).""" + project_root = tmp_path / "project" + project_root.mkdir() + + # Build a manifest by hand with main + a linked dev branch and no + # tracked configs -- the exact pre-condition Bug B blocks on. + keboola_dir = project_root / KEBOOLA_DIR_NAME + keboola_dir.mkdir() + (keboola_dir / "manifest.json").write_text( + json.dumps( + { + "version": MANIFEST_VERSION, + "project": {"id": 258, "apiHost": "connection.keboola.com"}, + "allowTargetEnv": True, + "gitBranching": {"enabled": True, "defaultBranch": "main"}, + "sortBy": "id", + "naming": { + "branch": "{branch_name}", + "config": "{component_type}/{component_id}/{config_name}", + "configRow": "rows/{config_row_name}", + "schedulerConfig": "schedules/{config_name}", + "sharedCodeConfig": "_shared/{target_component_id}", + "sharedCodeConfigRow": "codes/{config_row_name}", + "variablesConfig": "variables", + "variablesValuesRow": "values/{config_row_name}", + "dataAppConfig": "app/{component_id}/{config_name}", + }, + "allowedBranches": [], + "ignoredComponents": [], + "branches": [ + {"id": 12345, "path": "main", "metadata": {}}, + {"id": 99999, "path": "branch-99999", "metadata": {}}, + ], + "configurations": [], + } + ) + ) + + # Drop a scaffold under branch-99999/ + scaffold = ( + project_root / "branch-99999" / "application" / "test-component" / "my-test-config" + ) + scaffold.mkdir(parents=True) + (scaffold / CONFIG_FILENAME).write_text(yaml.safe_dump({"name": "my-test-config"})) + + manifest = load_manifest(project_root) + svc = SyncService(config_store=MagicMock()) + added = svc._find_untracked_configs( + project_root, + manifest, + resolved_branch_id=99999, + ) + + assert len(added) == 1 + assert added[0]["path"].endswith("my-test-config") + + def test_walker_skips_orphaned_branch_dirs(self, tmp_path: Path) -> None: + """Phantom-add protection still holds: a directory under a branch + whose id is neither tracked, default, nor explicitly resolved is + ignored. Guards against regression of Bug B in the wrong direction.""" + project_root = tmp_path / "project" + project_root.mkdir() + keboola_dir = project_root / KEBOOLA_DIR_NAME + keboola_dir.mkdir() + (keboola_dir / "manifest.json").write_text( + json.dumps( + { + "version": MANIFEST_VERSION, + "project": {"id": 258, "apiHost": "connection.keboola.com"}, + "allowTargetEnv": True, + "gitBranching": {"enabled": True, "defaultBranch": "main"}, + "sortBy": "id", + "naming": { + "branch": "{branch_name}", + "config": "{component_type}/{component_id}/{config_name}", + "configRow": "rows/{config_row_name}", + "schedulerConfig": "schedules/{config_name}", + "sharedCodeConfig": "_shared/{target_component_id}", + "sharedCodeConfigRow": "codes/{config_row_name}", + "variablesConfig": "variables", + "variablesValuesRow": "values/{config_row_name}", + "dataAppConfig": "app/{component_id}/{config_name}", + }, + "allowedBranches": [], + "ignoredComponents": [], + "branches": [ + {"id": 12345, "path": "main", "metadata": {}}, + {"id": 88888, "path": "branch-orphan", "metadata": {}}, + ], + "configurations": [], + } + ) + ) + + # Drop a scaffold under branch-orphan/ -- should be ignored because + # we resolve to branch 99999 (a different one). + scaffold = ( + project_root / "branch-orphan" / "application" / "test-component" / "ignored-config" + ) + scaffold.mkdir(parents=True) + (scaffold / CONFIG_FILENAME).write_text(yaml.safe_dump({"name": "ignored-config"})) + + manifest = load_manifest(project_root) + svc = SyncService(config_store=MagicMock()) + added = svc._find_untracked_configs( + project_root, + manifest, + resolved_branch_id=99999, # nothing on disk under this branch + ) + assert added == [] + + # ------------------------------------------------------------------ + # Bug E + # ------------------------------------------------------------------ + + def test_resolve_branch_id_default_branch_without_mapping( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """When on the default git branch, ``_resolve_branch_id`` returns + ``None`` (production) even if ``branch-mapping.json`` is missing + entirely. There is always a recovery path to production.""" + project_root = tmp_path / "project" + project_root.mkdir() + store = self._init_git_branching_project(tmp_config_dir, project_root) + + # Delete branch-mapping.json after init -- simulates accidental loss. + (project_root / KEBOOLA_DIR_NAME / BRANCH_MAPPING_FILENAME).unlink() + + manifest = load_manifest(project_root) + project = store.get_project("prod") + with patch( + "keboola_agent_cli.sync.git_utils.get_current_branch", + return_value="main", + ): + resolved = SyncService._resolve_branch_id(project, manifest, project_root) + assert resolved is None + + def test_resolve_branch_id_dev_branch_without_mapping_still_errors( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """The recovery path is intentionally narrow: only the default branch + gets the silent fall-through. A non-default branch with no mapping + still raises ``ConfigError`` so the user is told to link it.""" + project_root = tmp_path / "project" + project_root.mkdir() + store = self._init_git_branching_project(tmp_config_dir, project_root) + + (project_root / KEBOOLA_DIR_NAME / BRANCH_MAPPING_FILENAME).unlink() + + manifest = load_manifest(project_root) + project = store.get_project("prod") + with ( + patch( + "keboola_agent_cli.sync.git_utils.get_current_branch", + return_value="feature-x", + ), + pytest.raises(ConfigError, match="not linked"), + ): + SyncService._resolve_branch_id(project, manifest, project_root) diff --git a/uv.lock b/uv.lock index 7312ddab..6c58e458 100644 --- a/uv.lock +++ b/uv.lock @@ -439,7 +439,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.30.3" +version = "0.30.4" source = { editable = "." } dependencies = [ { name = "httpx" },