diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 4230621c..32f62d0c 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "kbagent", - "version": "0.43.0", + "version": "0.43.1", "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 2ad9340d..6f084261 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.43.0", + "version": "0.43.1", "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/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 8c957978..25fae127 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -108,8 +108,8 @@ a critical failure. user's local claude / codex / gemini CLI; backs the dashboard Local AI tile that replaces Kai for non-master-token projects) needs 0.41.9+, - data-app discoverability fix on `workspace list/detail` + - sandbox config annotation needs 0.42.0+ (#304; gotchas.md), + data-app workspace + CLI sandbox annotation = 0.42.0+ (#304), + HTTP opt-in `?include_sandbox_annotation=true` = 0.43.1+ (#312), `storage retype` is a future composite), you MUST refuse the task and return a handoff message to the parent: `"Cannot proceed safely on kbagent . Missing: . diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 4ed0a6ef..c57b760c 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -109,6 +109,18 @@ no workspace found". annotation to avoid N+1 (one `list_workspaces` per config). Use `workspace list --project NAME` as a one-shot lookup instead. +**HTTP / REST parity** (updated v0.43.1 -- closes #312): the annotation +now lives in `ConfigService.get_config_detail()` behind an opt-in +`include_sandbox_annotation: bool = False` parameter, not only in the +CLI command. `GET /configs/{project}/{component_id}/{config_id}` on +`kbagent serve` accepts `?include_sandbox_annotation=true` to switch it +on. Default off so existing programmatic / web UI consumers see the +unchanged shape -- a regression-free upgrade. The CLI command always +opts in to preserve v0.42.0 behavior. If `list_workspaces` fails (rate +limit, transient 5xx), the detail call still succeeds and +`storage_workspace_id` is set to `null` -- the annotation is UX, not a +contract. + ## Web UI `Kai Chat` is gone — replaced by `Local AI` (since v0.41.9) The web UI dashboard tile / left-nav entry previously labelled **Kai diff --git a/pyproject.toml b/pyproject.toml index c719f7a1..296b0434 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-agent-cli" -version = "0.43.0" +version = "0.43.1" 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 cf1e26ac..db938d23 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -8,6 +8,9 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.43.1": [ + "Fix: sandbox annotation from #304/#311 is now available to HTTP / REST callers, not only the CLI (closes #312). The original v0.42.0 fix placed the `keboola.sandboxes` `parameters.id` -> `storage_workspace_id` resolution in `commands/config.py` so it only fired on `kbagent config detail` invocations; `kbagent serve` callers (web UI, scheduled agents, `kbagent http get /configs/...`) hit the same `parameters.id` trap David Ešner reported in #304. The annotation logic moves to `ConfigService.get_config_detail` behind an opt-in `include_sandbox_annotation: bool = False` parameter so existing programmatic consumers see the unchanged response shape (zero regression risk), and a new `?include_sandbox_annotation=true` query parameter on `GET /configs/{project}/{component_id}/{config_id}` exposes it to REST callers. The CLI command unconditionally opts in to preserve v0.42.0 behavior. The pure-function workspace-list filter (`find_storage_workspace_for_sandbox_config(workspaces, config_id) -> int | None`) is extracted from `WorkspaceService.resolve_sandbox_workspace_id` to `services/workspace_service.py` module level so `ConfigService` can call it without taking a circular `ConfigService -> WorkspaceService` dependency in the DI graph. `WorkspaceService.resolve_sandbox_workspace_id` is retained as a one-line wrapper around the helper (still useful for direct callers). Error handling: a failed `list_workspaces` HTTP call no longer fails the detail fetch -- `storage_workspace_id` is set to `None` and the detail comes back as before, because the annotation is UX, not a contract. Bulk mode (`config_id=None`) silently ignores the flag because it would N+1 the workspace listing endpoint (one extra round-trip per config). Tests: 5 new in `test_services.py::TestConfigServiceSandboxAnnotation` covering default-off zero-regression, opt-in resolution, orphan (no matching workspace), non-sandbox-component skip, and graceful degradation on `list_workspaces` failure; 3 new in `test_serve_ui.py::TestConfigDetailSandboxAnnotation` covering HTTP router parameter binding (default-off, opt-in, non-sandbox no-op); 3 existing CLI tests in `test_cli.py::TestConfigDetail` updated to mock the new service-layer call path (now mock `client.list_workspaces` instead of `WorkspaceService.resolve_sandbox_workspace_id`). Total suite: 3381 passed, 104 skipped.", + ], "0.43.0": [ "New: full Semantic Layer management surface in `kbagent serve --ui` (closes #308). The web UI now mirrors every `kbagent semantic-layer` CLI operation 1:1 -- model CRUD (`/api/semantic-layer/models`), entity CRUD for all five kinds (metric / dataset / relationship / constraint / glossary), and the Phase-3 operations (validate, export, diff, promote, import, build, encrypt-token). The UI calls **zero** Metastore endpoints directly; every interaction goes through `/api/semantic-layer/*` on the same `kbagent serve` process so CLI parity is structural, not aspirational. Highlights: schema-driven add/edit drawers (one Pydantic schema per entity kind drives both Typer flags and the React form, no UI-side validation duplication); relationships view ships with a `flowchart TB` ERD (Mermaid) + a dataset-filter chip for hub-and-spoke drill-down + a parallel 'click to edit' edge list for hit-target reliability; constraints view groups rows by `constraintType` with collapsible `
` blocks and a 3-icon severity rail (critical / warning / info); datasets detail panel surfaces `fields[]` with role chips (`key=keboola/measure=green/dimension=zinc`). Builder/Importer/Promoter/Diff/Encrypt-Token are dedicated dialogs (`SemanticLayerDialogs.tsx`) with dry-run preview where the CLI offers it. The relationships ERD ships as `flowchart TB` (not `erDiagram`): erDiagram has no rankdir and laid every hub-and-spoke model out as a wide thin strip wasting ~70% of the canvas. flowchart TB puts the hub above its dependents, edge labels are trimmed to just the join type (`left` / `inner`; full relationship names live in the edge list below), auto-fit chooses `Math.min(fitX, fitY)` (cap 2.5, floor 0.4) so 80-edge overviews shrink to ~40% and 15-edge hub drill-downs land at ~63%, both fully readable.", 'Fix: `BaseHttpClient._raise_api_error` now correctly surfaces Metastore validation messages instead of printing a bare HTTP status code. The Keboola Metastore answers 422 with `{"error": 422, "description": "..."}` (int in `error`, real text in `description`); the old parser used `body.get("error")` as the priority key, which evaluated to `422` and shadowed the real message -- the CLI rendered `API error 422: 422` and the operator had no actionable text. The new walker accepts `error` ONLY when it is a non-empty string, then falls through to `exception → message → description → detail → errors → json.dumps(body)` in priority order; FastAPI\'s `{"detail": [{loc, msg}]}` and Metastore\'s `{"errors": [{loc, msg}]}` list shapes are json-serialised so the message contains every diagnostic line, not the Python list repr. Four regression tests pin the new paths (int `error`, plain `description`, both list shapes) so the bare-status-code UX cannot return silently.', diff --git a/src/keboola_agent_cli/commands/config.py b/src/keboola_agent_cli/commands/config.py index b4aa5e16..4cedcb0f 100644 --- a/src/keboola_agent_cli/commands/config.py +++ b/src/keboola_agent_cli/commands/config.py @@ -291,16 +291,25 @@ def config_detail( try: if config_id is not None: - # Single-config mode: shape unchanged for backward compat + # Single-config mode: shape unchanged for backward compat, + # plus the opt-in sandbox annotation (since v0.42.1, issue #312): + # the service layer now owns the keboola.sandboxes + # configurationId->workspace.id resolution so HTTP and REST + # callers get the same enrichment, not only the CLI. result = service.get_config_detail( alias=project[0], component_id=component_id, config_id=config_id, branch_id=effective_branch, with_state=with_state, + include_sandbox_annotation=True, ) else: - # Bulk mode: one call per project, filtered by component_id + # Bulk mode: one call per project, filtered by component_id. + # Annotation flag stays off here -- bulk mode would N+1 the + # workspace listing endpoint (one extra round-trip per config), + # and the field that triggers the annotation in single-config + # mode (parameters.id) is rarely consumed in bulk anyway. result = service.get_config_detail( alias=project[0], component_id=component_id, @@ -322,39 +331,6 @@ def config_detail( ) raise typer.Exit(code=exit_code) from None - # Issue #304 bod #3: ``keboola.sandboxes`` configs carry ``parameters.id`` - # that looks like a Storage workspace ID but is actually a sandbox-service - # internal handle (passing it to ``workspace detail --workspace-id`` 404s). - # Resolve the real Storage workspace ID once via WorkspaceService so JSON - # callers get the mapping and human-mode readers see an explicit annotation. - # Single-config mode only -- bulk mode is N+1-sensitive (one extra request - # per config) and would be a regression for the existing fast-fan-out use. - if config_id is not None and component_id == "keboola.sandboxes": - ws_service = get_service(ctx, "workspace_service") - configuration = result.get("configuration", {}) or {} - sandbox_service_id = (configuration.get("parameters") or {}).get("id") - try: - storage_workspace_id = ws_service.resolve_sandbox_workspace_id( - alias=project[0], - config_id=config_id, - branch_id=effective_branch, - ) - except (KeboolaApiError, ConfigError): - # Best-effort enrichment: do not fail the whole detail call just - # because the workspace listing endpoint hiccuped. The annotation - # is a UX nicety, not a contract. - storage_workspace_id = None - result["sandbox_annotation"] = { - "sandbox_service_id": sandbox_service_id, - "storage_workspace_id": storage_workspace_id, - "note": ( - "`parameters.id` in a keboola.sandboxes config is the " - "sandbox-service internal ID, NOT the Storage workspace ID. " - "Use `storage_workspace_id` with `kbagent workspace detail " - "--workspace-id ...`." - ), - } - if config_id is not None: # Single-config mode: emit unchanged shape formatter.output(result, format_config_detail) diff --git a/src/keboola_agent_cli/server/routers/configs.py b/src/keboola_agent_cli/server/routers/configs.py index 7e799a84..491c3f9a 100644 --- a/src/keboola_agent_cli/server/routers/configs.py +++ b/src/keboola_agent_cli/server/routers/configs.py @@ -111,6 +111,18 @@ def config_detail( config_id: str, branch_id: int | None = None, with_state: bool = False, + include_sandbox_annotation: bool = Query( + False, + description=( + "Opt-in enrichment for component_id=keboola.sandboxes. When true, " + "the response carries a `sandbox_annotation` block with " + "`sandbox_service_id` (the misleading `configuration.parameters.id`) " + "and `storage_workspace_id` (the actual Storage workspace ID, " + "resolved via an extra GET /v2/storage/workspaces). Off by default " + "to keep the endpoint response shape stable for existing callers. " + "Closes #312 (HTTP parity for the #304 trap)." + ), + ), registry: ServiceRegistry = Depends(get_registry), ) -> dict[str, Any]: """Fetch a single configuration. Mirrors `kbagent config detail`.""" @@ -120,6 +132,7 @@ def config_detail( config_id=config_id, branch_id=branch_id, with_state=with_state, + include_sandbox_annotation=include_sandbox_annotation, ) diff --git a/src/keboola_agent_cli/services/config_service.py b/src/keboola_agent_cli/services/config_service.py index 1a2a8baa..789e1d77 100644 --- a/src/keboola_agent_cli/services/config_service.py +++ b/src/keboola_agent_cli/services/config_service.py @@ -23,6 +23,7 @@ from ..sync.manifest import Manifest, load_manifest, save_manifest from ..sync.naming import sanitize_name from .base import BaseService, ClientFactory, sanitize_unexpected_error +from .workspace_service import find_storage_workspace_for_sandbox_config AiClientFactory = Callable[[str, str], AiServiceClient] @@ -299,6 +300,7 @@ def get_config_detail( branch_id: int | None = None, with_state: bool = False, aliases: list[str] | None = None, + include_sandbox_annotation: bool = False, ) -> dict[str, Any]: """Get detailed information about one or many configurations. @@ -340,6 +342,18 @@ def get_config_detail( ``config_id`` must be None and ``branch_id`` must be None. Returns ``{"configs": [...], "errors": [...]}`` with every row tagged by ``project_alias``. + include_sandbox_annotation: Opt-in enrichment for + ``component_id == "keboola.sandboxes"`` in single-config + mode. When True, the response gains a + ``sandbox_annotation`` block with ``sandbox_service_id`` + (the misleading ``configuration.parameters.id``) and + ``storage_workspace_id`` (the actual Storage workspace ID, + resolved via an extra ``GET /v2/storage/workspaces``). + Default False to keep this method a clean API wrapper for + programmatic callers (closes #312 -- HTTP/REST parity gap + left by #304). Bulk mode is N+1-sensitive (one extra HTTP + round-trip per config), so the flag is silently ignored + there. Returns: Dict. Shape depends on mode: @@ -403,11 +417,45 @@ def get_config_detail( detail.setdefault("state", {}) if not isinstance(detail["state"], dict): detail["state"] = {} + # Sandbox annotation enrichment (issue #312 / #304 HTTP parity). + # Opt-in (default off) so existing programmatic consumers keep + # the unchanged shape. The extra ``list_workspaces`` HTTP call + # is intentional: there is no per-config sandbox→workspace + # endpoint, and reusing the same client keeps retry/backoff + + # branch routing consistent with the detail call above. + sandbox_annotation: dict[str, Any] | None = None + if include_sandbox_annotation and component_id == "keboola.sandboxes": + sandbox_service_id = ( + (detail.get("configuration") or {}).get("parameters", {}).get("id") + ) + try: + workspaces = client.list_workspaces(branch_id=effective_branch_id) + storage_workspace_id = find_storage_workspace_for_sandbox_config( + workspaces, config_id + ) + except KeboolaApiError: + # Best-effort: do not fail the detail fetch just because + # the workspace listing endpoint hiccuped -- the + # annotation is a UX nicety, not a contract. The caller + # still gets the raw detail. + storage_workspace_id = None + sandbox_annotation = { + "sandbox_service_id": sandbox_service_id, + "storage_workspace_id": storage_workspace_id, + "note": ( + "`parameters.id` in a keboola.sandboxes config is the " + "sandbox-service internal ID, NOT the Storage workspace ID. " + "Use `storage_workspace_id` with `kbagent workspace detail " + "--workspace-id ...`." + ), + } finally: client.close() detail["project_alias"] = alias detail["branch_id"] = effective_branch_id + if sandbox_annotation is not None: + detail["sandbox_annotation"] = sandbox_annotation return detail def _get_config_detail_bulk( diff --git a/src/keboola_agent_cli/services/workspace_service.py b/src/keboola_agent_cli/services/workspace_service.py index a1cbbd0c..5c2601ba 100644 --- a/src/keboola_agent_cli/services/workspace_service.py +++ b/src/keboola_agent_cli/services/workspace_service.py @@ -28,6 +28,49 @@ def _classify_qs_compatibility(login_type: str) -> bool: return login_type in QUERY_SERVICE_COMPATIBLE_LOGIN_TYPES +def find_storage_workspace_for_sandbox_config( + workspaces: list[dict[str, Any]], + config_id: str, +) -> int | None: + """Pure-function lookup: find the Storage workspace that backs a sandbox config. + + A ``keboola.sandboxes`` configuration's ``parameters.id`` is the + sandbox-service internal ID, not a Storage workspace ID -- passing it to + ``GET /v2/storage/workspaces/{ID}`` returns 404 (issue #304). The real + relation goes the other direction: each Storage workspace exposes + ``configurationId`` pointing back at its sandbox config. + + Extracted from ``WorkspaceService.resolve_sandbox_workspace_id`` so + ``ConfigService.get_config_detail`` can call it with a workspace list it + already has (avoiding a circular ``ConfigService -> WorkspaceService`` + dependency and the extra HTTP round-trip that would otherwise pile up + in HTTP and web-UI consumers -- see issue #312). + + Args: + workspaces: Raw output of ``KeboolaClient.list_workspaces()`` -- each + entry is the Storage API workspace dict (not the normalised CLI + shape). + config_id: ``keboola.sandboxes`` configuration ID. + + Returns: + Storage workspace ID (int), or None if no workspace currently backs + this config (orphan sandbox, or workspace deleted but config kept + around). + """ + for ws in workspaces: + if ws.get("component") == "keboola.sandboxes" and str(ws.get("configurationId", "")) == str( + config_id + ): + ws_id = ws.get("id") + if isinstance(ws_id, int): + return ws_id + try: + return int(ws_id) if ws_id is not None else None + except (TypeError, ValueError): + return None + return None + + def _is_orphaned_workspace(ws: dict[str, Any], config_names: dict[str, str]) -> bool: """Return True if a workspace has no backing keboola.sandboxes config. @@ -321,18 +364,7 @@ def resolve_sandbox_workspace_id( finally: client.close() - for ws in workspaces: - if ws.get("component") == "keboola.sandboxes" and str( - ws.get("configurationId", "") - ) == str(config_id): - ws_id = ws.get("id") - if isinstance(ws_id, int): - return ws_id - try: - return int(ws_id) if ws_id is not None else None - except (TypeError, ValueError): - return None - return None + return find_storage_workspace_for_sandbox_config(workspaces, config_id) def list_workspaces( self, diff --git a/tests/test_cli.py b/tests/test_cli.py index ca4a1418..ccb39590 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1812,9 +1812,12 @@ def test_config_detail_sandbox_annotation(self, tmp_path: Path) -> None: Regression test for issue #304 bod #3 -- a sandbox config's ``parameters.id`` looks like a Storage workspace ID but is in fact - the sandbox-service-internal handle. The command layer must enrich - the detail with a ``sandbox_annotation`` block containing the - resolved Storage workspace ID looked up via WorkspaceService. + the sandbox-service-internal handle. Since v0.42.1 (issue #312) + the enrichment lives in ``ConfigService.get_config_detail``, so + this test mocks the Storage client's ``list_workspaces`` call to + return a workspace whose ``configurationId`` matches the sandbox + config; the service walks the list to resolve the real workspace + id. """ config_dir = tmp_path / "config" config_dir.mkdir() @@ -1828,20 +1831,23 @@ def test_config_detail_sandbox_annotation(self, tmp_path: Path) -> None: } mock_client = MagicMock() mock_client.get_config_detail.return_value = detail_response + mock_client.list_workspaces.return_value = [ + { + "id": 2950518214, + "component": "keboola.sandboxes", + "configurationId": "sb-cfg-1", + }, + ] store = _setup_config_test( config_dir, {"prod": {"token": "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k"}}, ) - mock_ws = MagicMock() - mock_ws.resolve_sandbox_workspace_id.return_value = 2950518214 - with ( patch("keboola_agent_cli.cli.ConfigStore") as MockStore, patch("keboola_agent_cli.cli.ProjectService") as MockProjService, patch("keboola_agent_cli.cli.ConfigService") as MockCfgService, - patch("keboola_agent_cli.cli.WorkspaceService") as MockWsService, ): MockStore.return_value = store MockProjService.return_value = ProjectService(config_store=store) @@ -1849,7 +1855,6 @@ def test_config_detail_sandbox_annotation(self, tmp_path: Path) -> None: config_store=store, client_factory=lambda url, token: mock_client, ) - MockWsService.return_value = mock_ws result = runner.invoke( app, @@ -1873,9 +1878,7 @@ def test_config_detail_sandbox_annotation(self, tmp_path: Path) -> None: assert annotation["sandbox_service_id"] == "1296392806" assert annotation["storage_workspace_id"] == 2950518214 assert "sandbox-service internal ID" in annotation["note"] - mock_ws.resolve_sandbox_workspace_id.assert_called_once_with( - alias="prod", config_id="sb-cfg-1", branch_id=None - ) + mock_client.list_workspaces.assert_called_once() def test_config_detail_sandbox_annotation_orphan(self, tmp_path: Path) -> None: """When no workspace is currently backed by the sandbox config, the annotation @@ -1894,20 +1897,25 @@ def test_config_detail_sandbox_annotation_orphan(self, tmp_path: Path) -> None: } mock_client = MagicMock() mock_client.get_config_detail.return_value = detail_response + # No workspace points at sb-cfg-orphan -> service returns None for + # storage_workspace_id without raising. + mock_client.list_workspaces.return_value = [ + { + "id": 9999, + "component": "keboola.sandboxes", + "configurationId": "some-other-config", + }, + ] store = _setup_config_test( config_dir, {"prod": {"token": "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k"}}, ) - mock_ws = MagicMock() - mock_ws.resolve_sandbox_workspace_id.return_value = None - with ( patch("keboola_agent_cli.cli.ConfigStore") as MockStore, patch("keboola_agent_cli.cli.ProjectService") as MockProjService, patch("keboola_agent_cli.cli.ConfigService") as MockCfgService, - patch("keboola_agent_cli.cli.WorkspaceService") as MockWsService, ): MockStore.return_value = store MockProjService.return_value = ProjectService(config_store=store) @@ -1915,7 +1923,6 @@ def test_config_detail_sandbox_annotation_orphan(self, tmp_path: Path) -> None: config_store=store, client_factory=lambda url, token: mock_client, ) - MockWsService.return_value = mock_ws result = runner.invoke( app, @@ -1960,13 +1967,10 @@ def test_config_detail_no_sandbox_annotation_for_non_sandbox_component( {"prod": {"token": "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k"}}, ) - mock_ws = MagicMock() - with ( patch("keboola_agent_cli.cli.ConfigStore") as MockStore, patch("keboola_agent_cli.cli.ProjectService") as MockProjService, patch("keboola_agent_cli.cli.ConfigService") as MockCfgService, - patch("keboola_agent_cli.cli.WorkspaceService") as MockWsService, ): MockStore.return_value = store MockProjService.return_value = ProjectService(config_store=store) @@ -1974,7 +1978,6 @@ def test_config_detail_no_sandbox_annotation_for_non_sandbox_component( config_store=store, client_factory=lambda url, token: mock_client, ) - MockWsService.return_value = mock_ws result = runner.invoke( app, @@ -1994,7 +1997,9 @@ def test_config_detail_no_sandbox_annotation_for_non_sandbox_component( assert result.exit_code == 0 data = json.loads(result.output)["data"] assert "sandbox_annotation" not in data - mock_ws.resolve_sandbox_workspace_id.assert_not_called() + # Service must NOT fan out to list_workspaces for non-sandbox components, + # even with include_sandbox_annotation=True (which the CLI sets). + mock_client.list_workspaces.assert_not_called() def test_config_detail_human_output(self, tmp_path: Path) -> None: """config detail in human mode shows a Rich panel with details.""" diff --git a/tests/test_serve_ui.py b/tests/test_serve_ui.py index 4e1305d6..f9cab128 100644 --- a/tests/test_serve_ui.py +++ b/tests/test_serve_ui.py @@ -190,3 +190,98 @@ def test_missing_dist_skips_mount_silently( resp = client.get("/", headers={"authorization": "Bearer t"}) assert resp.status_code == 404 assert any("missing index.html" in rec.message for rec in caplog.records) + + +class TestConfigDetailSandboxAnnotation: + """Issue #312: ``include_sandbox_annotation=true`` query param on + ``GET /configs/{project}/{component_id}/{config_id}`` triggers the + same enrichment as the CLI ``config detail`` command. + + Validates the HTTP-side closure of the #304 trap: web UI / scheduled + agent / third-party HTTP callers can now see ``sandbox_annotation`` + without spelunking through ``parameters.id``. + + Default-off contract is also pinned: without the flag, the response + shape is unchanged so existing programmatic consumers stay isolated + from the new field. + """ + + def _patch_config_service(self, app, *, with_workspace: bool) -> dict: + """Replace the real ConfigService.get_config_detail with a stub. + + Avoids hitting any real Keboola HTTP -- the test focuses on + FastAPI's parameter binding and the router -> service plumbing. + """ + captured: dict = {} + + def fake_get_config_detail(**kwargs): + captured["kwargs"] = kwargs + response = { + "id": kwargs["config_id"], + "componentId": kwargs["component_id"], + "configuration": {"parameters": {"id": "1296392806"}}, + "rows": [], + "project_alias": kwargs["alias"], + "branch_id": kwargs.get("branch_id"), + } + # Emulate what the real service does when include_sandbox_annotation + # is true and the component is keboola.sandboxes. + if ( + kwargs.get("include_sandbox_annotation") + and kwargs["component_id"] == "keboola.sandboxes" + ): + response["sandbox_annotation"] = { + "sandbox_service_id": "1296392806", + "storage_workspace_id": 2950518214 if with_workspace else None, + "note": "`parameters.id` ... sandbox-service internal ID ...", + } + return response + + app.state.registry.config.get_config_detail = fake_get_config_detail # type: ignore[method-assign] + return captured + + def test_flag_off_keeps_response_shape_stable(self, tmp_path: Path) -> None: + """Default GET (no query param) MUST NOT include sandbox_annotation.""" + client = _make_client(tmp_path, token="t") + captured = self._patch_config_service(client.app, with_workspace=True) + + resp = client.get( + "/configs/prod/keboola.sandboxes/sb-cfg-1", + headers={"authorization": "Bearer t"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert "sandbox_annotation" not in body + # Router forwarded include_sandbox_annotation=False (FastAPI default). + assert captured["kwargs"]["include_sandbox_annotation"] is False + + def test_flag_on_returns_annotation(self, tmp_path: Path) -> None: + """`?include_sandbox_annotation=true` propagates through the router.""" + client = _make_client(tmp_path, token="t") + captured = self._patch_config_service(client.app, with_workspace=True) + + resp = client.get( + "/configs/prod/keboola.sandboxes/sb-cfg-1", + params={"include_sandbox_annotation": "true"}, + headers={"authorization": "Bearer t"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + ann = body.get("sandbox_annotation") + assert ann is not None, "expected sandbox_annotation in HTTP response" + assert ann["sandbox_service_id"] == "1296392806" + assert ann["storage_workspace_id"] == 2950518214 + assert captured["kwargs"]["include_sandbox_annotation"] is True + + def test_flag_on_for_non_sandbox_component_is_no_op(self, tmp_path: Path) -> None: + """The flag is keboola.sandboxes-specific; other components stay clean.""" + client = _make_client(tmp_path, token="t") + self._patch_config_service(client.app, with_workspace=True) + + resp = client.get( + "/configs/prod/keboola.ex-db-snowflake/cfg-101", + params={"include_sandbox_annotation": "true"}, + headers={"authorization": "Bearer t"}, + ) + assert resp.status_code == 200, resp.text + assert "sandbox_annotation" not in resp.json() diff --git a/tests/test_services.py b/tests/test_services.py index eed87a7c..5507c606 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -2045,6 +2045,208 @@ def test_unexpected_error_message_short_not_truncated(self, tmp_config_dir: Path assert result["errors"][0]["message"] == "boom" +class TestConfigServiceSandboxAnnotation: + """Tests for ConfigService.get_config_detail(include_sandbox_annotation=...). + + Verifies the issue #312 service-layer enrichment that closes the HTTP/REST + parity gap left by #304 (CLI-only). The flag defaults False so existing + programmatic / HTTP callers see the unchanged shape; opting in resolves + the sandbox config's misleading parameters.id to the real Storage + workspace ID via an additional list_workspaces call. + """ + + def _make_store(self, tmp_config_dir: Path) -> ConfigStore: + store = ConfigStore(config_dir=tmp_config_dir) + store.add_project( + "prod", + ProjectConfig( + stack_url="https://connection.keboola.com", + token="901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k", + ), + ) + return store + + def test_annotation_off_by_default(self, tmp_config_dir: Path) -> None: + """Default include_sandbox_annotation=False keeps the response shape stable. + + This is the contract for existing programmatic callers (web UI, scheduled + agents, third-party scripts) -- they must NOT see a new key appear in the + response just because they upgraded kbagent. + """ + store = self._make_store(tmp_config_dir) + detail = { + "id": "sb-cfg-1", + "name": "RO sandbox", + "componentId": "keboola.sandboxes", + "configuration": {"parameters": {"id": "1296392806"}}, + "rows": [], + } + mock_client = MagicMock() + mock_client.get_config_detail.return_value = detail + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + + result = service.get_config_detail( + alias="prod", + component_id="keboola.sandboxes", + config_id="sb-cfg-1", + ) + + assert "sandbox_annotation" not in result + # No fan-out to list_workspaces when the flag is off; this is the + # zero-regression guarantee for callers that don't opt in. + mock_client.list_workspaces.assert_not_called() + # Client must be closed exactly once even on the fast path so the + # connection pool doesn't leak. Each annotation test pins this so + # an accidental early-return that skips the finally would be caught. + mock_client.close.assert_called_once() + + def test_annotation_resolves_storage_workspace_id(self, tmp_config_dir: Path) -> None: + """With the flag on, the service resolves the real Storage workspace ID.""" + store = self._make_store(tmp_config_dir) + detail = { + "id": "sb-cfg-1", + "name": "RO sandbox", + "componentId": "keboola.sandboxes", + "configuration": {"parameters": {"id": "1296392806"}}, + "rows": [], + } + mock_client = MagicMock() + mock_client.get_config_detail.return_value = detail + mock_client.list_workspaces.return_value = [ + { + "id": 2950518214, + "component": "keboola.sandboxes", + "configurationId": "sb-cfg-1", + }, + ] + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + + result = service.get_config_detail( + alias="prod", + component_id="keboola.sandboxes", + config_id="sb-cfg-1", + include_sandbox_annotation=True, + ) + + ann = result["sandbox_annotation"] + assert ann["sandbox_service_id"] == "1296392806" + assert ann["storage_workspace_id"] == 2950518214 + assert "sandbox-service internal ID" in ann["note"] + # branch_id is forwarded to list_workspaces so the lookup hits the + # same scope as the detail call. + mock_client.list_workspaces.assert_called_once_with(branch_id=None) + mock_client.close.assert_called_once() + + def test_annotation_orphan_returns_none_for_workspace_id(self, tmp_config_dir: Path) -> None: + """No workspace currently backs the config -> storage_workspace_id is None, + annotation block still present so callers can distinguish 'no annotation + ran' from 'annotation ran but no workspace found'.""" + store = self._make_store(tmp_config_dir) + detail = { + "id": "sb-orphan", + "name": "Orphan sandbox", + "componentId": "keboola.sandboxes", + "configuration": {}, # no parameters.id at all + "rows": [], + } + mock_client = MagicMock() + mock_client.get_config_detail.return_value = detail + mock_client.list_workspaces.return_value = [] + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + + result = service.get_config_detail( + alias="prod", + component_id="keboola.sandboxes", + config_id="sb-orphan", + include_sandbox_annotation=True, + ) + + ann = result["sandbox_annotation"] + assert ann["sandbox_service_id"] is None + assert ann["storage_workspace_id"] is None + mock_client.close.assert_called_once() + + def test_annotation_skipped_for_non_sandbox_component(self, tmp_config_dir: Path) -> None: + """Annotation is keboola.sandboxes-specific; other components must NOT + trigger the extra list_workspaces fan-out even when the flag is on.""" + store = self._make_store(tmp_config_dir) + detail = { + "id": "cfg-101", + "name": "Snowflake extractor", + "componentId": "keboola.ex-db-snowflake", + "configuration": {"parameters": {"db": "prod"}}, + "rows": [], + } + mock_client = MagicMock() + mock_client.get_config_detail.return_value = detail + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + + result = service.get_config_detail( + alias="prod", + component_id="keboola.ex-db-snowflake", + config_id="cfg-101", + include_sandbox_annotation=True, + ) + + assert "sandbox_annotation" not in result + mock_client.list_workspaces.assert_not_called() + mock_client.close.assert_called_once() + + def test_annotation_swallows_workspace_listing_error(self, tmp_config_dir: Path) -> None: + """If list_workspaces fails (rate limit, transient 5xx, ...), the detail + call MUST still succeed -- the annotation is UX, not contract. The + annotation block appears with storage_workspace_id=None so callers + can tell the difference between 'orphan' and 'lookup failed'.""" + from keboola_agent_cli.errors import KeboolaApiError + + store = self._make_store(tmp_config_dir) + detail = { + "id": "sb-cfg-1", + "componentId": "keboola.sandboxes", + "configuration": {"parameters": {"id": "1296392806"}}, + } + mock_client = MagicMock() + mock_client.get_config_detail.return_value = detail + mock_client.list_workspaces.side_effect = KeboolaApiError( + message="rate limited", + status_code=429, + error_code="RATE_LIMITED", + retryable=True, + ) + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + + result = service.get_config_detail( + alias="prod", + component_id="keboola.sandboxes", + config_id="sb-cfg-1", + include_sandbox_annotation=True, + ) + + ann = result["sandbox_annotation"] + assert ann["sandbox_service_id"] == "1296392806" + assert ann["storage_workspace_id"] is None + # The finally block must still run even when list_workspaces raised + # mid-try, otherwise an httpx client leaks per call. This pins the + # exception-safe close path that the bare try/except KeboolaApiError + # would otherwise let regress silently. + mock_client.close.assert_called_once() + + class TestConfigServiceListConfigsIncludeRows: """Tests for ConfigService.list_configs(include_rows=...).""" diff --git a/uv.lock b/uv.lock index 582ba8ce..35e8c375 100644 --- a/uv.lock +++ b/uv.lock @@ -496,7 +496,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.43.0" +version = "0.43.1" source = { editable = "." } dependencies = [ { name = "httpx" },