From a4e7fcc5a7c836236698c83de98787703e060c37 Mon Sep 17 00:00:00 2001 From: soustruh Date: Wed, 30 Sep 2026 03:58:16 +0200 Subject: [PATCH 1/2] feat(sync): opt-in sync of shared SQL workspaces in pull, diff, push and clone (CLI-25) --- CLAUDE.md | 6 +- docs/TUTORIAL.md | 2 + plugins/kbagent/agents/keboola-expert.md | 5 +- .../kbagent/references/commands-reference.md | 6 +- .../skills/kbagent/references/gotchas.md | 35 + .../references/permissions-workflow.md | 1 + .../kbagent/references/sync-workflow.md | 76 +- src/keboola_agent_cli/client/_client.py | 4 +- src/keboola_agent_cli/client/_core.py | 21 + src/keboola_agent_cli/client/editor.py | 73 + .../commands/_sync_push_render.py | 14 +- src/keboola_agent_cli/commands/context.py | 25 +- src/keboola_agent_cli/commands/sync.py | 28 +- src/keboola_agent_cli/constants.py | 2 +- src/keboola_agent_cli/permissions.py | 6 + .../services/_sync_baseline.py | 2 +- .../services/_sync_clone_warnings.py | 16 +- .../services/_sync_push_ops.py | 24 +- src/keboola_agent_cli/services/_sync_stale.py | 29 +- .../services/_sync_workspace.py | 461 ++++++ .../services/sync_service.py | 44 +- src/keboola_agent_cli/sync/manifest.py | 9 +- tests/test_sync_clone_links.py | 47 + tests/test_sync_workspaces.py | 1374 +++++++++++++++++ 24 files changed, 2256 insertions(+), 54 deletions(-) create mode 100644 src/keboola_agent_cli/client/editor.py create mode 100644 src/keboola_agent_cli/services/_sync_workspace.py create mode 100644 tests/test_sync_workspaces.py diff --git a/CLAUDE.md b/CLAUDE.md index 238c2581b..33379b7ec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -82,7 +82,7 @@ src/keboola_agent_cli/ http_base.py # BaseHttpClient - shared retry/backoff + common HTTP infra client/ # Storage API + Queue API package (X-StorageApi-Token); # split by endpoint family (storage_tables/storage_files/configs/ - # queue/tokens/branches/stream/query/workspaces/misc + _core/_transfer), + # queue/tokens/branches/stream/query/workspaces/editor/misc + _core/_transfer), # composed into one KeboolaClient via mixins (#520) manage_client.py # Manage API (X-KBC-ManageApiToken) ai_client.py # AI Service API (component schemas, Kai) @@ -954,7 +954,8 @@ kbagent config new --component-id ID [--name NAME] [--project NAME] [--output-di # mirrors the pushed encrypted body -- placeholders would overwrite the remote on next push. # sync: GitOps -- configs as local files. init/pull/push/diff are filesystem-local (no serve REST surface). -kbagent sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing] +kbagent sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing] [--with-workspaces] +# `sync init --with-workspaces` (CLI-25) sets the manifest key `syncWorkspaces`: pull/diff/push/clone then also sync shared SQL workspaces (keboola.sandboxes with no parameters.id and runtime.shared true; Python/R and legacy SQL sandboxes stay skipped), config only (services/_sync_workspace.py). A `push --force` delete removes the workspace's SQL editor sessions (Editor service, client/editor.py) before the config, `push --dry-run --force` lists them, and a plain push holds the delete back under skipped_deletions; clone warns about workspace input tables missing in the target. With --adopt-existing it turns the key on in an existing manifest. Version gate for this entry lives in gotchas.md. kbagent sync pull --project ALIAS [--all-projects] [--force] [--theirs] [--dry-run] [--with-samples] [--no-storage] [--no-jobs] [--job-limit N] [--branch ID] # `sync pull` auto-inits: if the target directory has no `.keboola/manifest.json`, pull runs `init` first, so a separate `sync init` is NOT needed for a first checkout. `sync pull --project X -d ./dir` on an empty dir writes the manifest and fetches the configs in one step. # `sync pull --force` is conflict-aware (since 0.53.0): locally-modified config whose remote is UNCHANGED is preserved (delta stays pushable, never silently re-stamped); a true merge conflict (local AND remote both changed since last pull) aborts (exit 1, SYNC_CONFLICT, --json lists details.conflicts); local-untouched + remote-changed takes remote. @@ -966,6 +967,7 @@ kbagent sync push --project ALIAS [--all-projects] [--dry-run] [--force] [--allo # under skipped_deletions (+ skipped_deletions_reason), also in --dry-run, whose summary.deleted counts only # what push would delete. A config/row deleted on the remote since the last pull diffs as remote_deleted and # is never re-created (it lands in skipped). Version gate in gotchas.md. +# sync push workspace delete (CLI-25): in a `syncWorkspaces` tree, a `push --force` that deletes a shared SQL workspace also deletes its SQL editor sessions (every user's, push branch) and their backend workspaces, which `config restore` does not bring back; `push --dry-run --force` lists them (warnings[] workspace_sessions), a plain push lists the workspace under skipped_deletions and touches no session. `--force` is destructive-class (FLAG_ESCALATIONS `sync.push --force`), so `--deny-destructive` / a cli:destructive deny blocks it while a plain push stays write-class. Version gate for this entry lives in gotchas.md. # sync push (since 0.91.0, #686): the manifest baseline `pull_config_hash` is stamped from the API # response (or a read-back), never from disk -- push-deployed multi-statement SQL transformations # (and anything disabled in the UI whose local YAML lacks `is_disabled`) no longer show permanent diff --git a/docs/TUTORIAL.md b/docs/TUTORIAL.md index 1c06a7c31..53f421371 100644 --- a/docs/TUTORIAL.md +++ b/docs/TUTORIAL.md @@ -512,6 +512,8 @@ git add -A && git commit -m "initial sync" `manifest.json`'s `ignoredComponents` field (since 0.91.0) lets you exclude project-specific components from every sync operation, on top of the always-ignored `keboola.sandboxes` and `keboola.mcp-server-tool`. +`sync init --with-workspaces` *(since vNEXT)* opts a tree in to syncing its +shared SQL workspaces (`keboola.sandboxes`), config only. What you end up with on disk: diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index 8a986ec01..60862006c 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -322,7 +322,10 @@ its absence is NOT a promise the entry is version-independent (see ยง1 Rule 6). action `"ignored"`, distinct from `"removed"`); a stale local dir for an already-ignored component can never classify as `DELETED` -- so delete-dir-then-push is safe for those, but on <= 0.90.1 it still deletes - the config in production. + the config in production. Exception (vNEXT+): a manifest with + `"syncWorkspaces": true` (`sync init --with-workspaces`) syncs shared SQL + workspaces, config only; a `push --force` delete of one also deletes its SQL + editor sessions (every user's); run `push --dry-run --force` first, it lists them. - **Native types**: `--column amount:NUMBER(18,2)` passes through; `BOOLEAN` defaults must be lowercase; `INTEGER(10)` is invalid (use `NUMBER(3,0)`); `--not-null` / `--default` must name a defined `--column`. In a dev branch diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index cb3b0d23c..f4553fc79 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -358,9 +358,9 @@ Requires the project to be added with its **master ('owner') Storage API token** - Exposed over `kbagent serve` as `GET /notifications`, `GET /notifications/{project}/{subscription_id}`, `POST /notifications/{project}`, `DELETE /notifications/{project}/{subscription_id}`, and `POST /notifications/{project}/{subscription_id}/replace-recipient` ## Sync (GitOps) -- `sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing]` -- initialize sync working directory; `--adopt-existing` adopts a `.keboola/manifest.json` already written by the kbc Go CLI without overwriting (idempotent; validates `project_id` against the alias token) -- `sync pull --project ALIAS [--all-projects] [--force] [--theirs] [--dry-run] [--with-samples] [--no-storage] [--no-jobs] [--job-limit N] [--branch ID]` -- download configs to local files. **Auto-inits:** if the target directory has no `.keboola/manifest.json`, pull runs `init` first, so a separate `sync init` is not needed for a first checkout of a project. For large projects (>100 configs), automatically fetches jobs per-config when the grouped API limit is insufficient. `--force` is conflict-aware (since 0.53.0): a locally-modified config whose remote is unchanged is **preserved** (pending delta stays pushable, never silently re-stamped); a true merge conflict (local AND remote both changed since last pull) **aborts** the pull (exit 1, `SYNC_CONFLICT`; `--json` lists `details.conflicts`); local-untouched + remote-changed takes remote. `--theirs` (since v0.72.0) is the supported "discard local, take production" reconcile path: overwrites locally-modified configs/rows, restores deleted/missing files, resolves conflicts by taking remote (no abort, no manifest surgery). Since v0.72.0 plain pull also re-materializes a tracked config whose local dir was deleted (manifest<->disk invariant), so delete-dir-then-pull refetches. Config-level `isDisabled` round-trips (since v0.72.0) as sparse `is_disabled: true` in `_config.yml` -- absent key = enabled. `--branch` (0.47.0+) per-invocation dev-branch override, beats every other branch source. Ignored components (since 0.91.0): `keboola.sandboxes` + `keboola.mcp-server-tool` are always excluded, unioned with the manifest's `ignoredComponents` list; a component newly ignored has its manifest entry dropped and local directory removed, reported with pull action `"ignored"` (distinct from `"removed"` = genuinely deleted on remote). Config-folder round-trip *(since 0.94.0)*: pull captures each config's UI folder (`KBC.configuration.folderName`, from the branch-only `search/component-configurations` endpoint) into the manifest, and reports `folder_lookup_failed` when that lookup fails, keeping the previously captured folder. -- `sync push --project ALIAS [--all-projects] [--dry-run] [--force] [--allow-plaintext-on-encrypt-failure] [--branch ID] [--no-name-drift-warnings]` -- push local changes (auto-encrypts secrets, fails if encryption fails). Fresh-CREATE writeback updates placeholder manifest entries in place (since 0.47.0) and propagates any `KBC.configuration.*` metadata via `set_config_metadata`. Fresh-CREATE variable binding (since 0.47.2): when a `keboola.variables` config + its values row are created alongside a transformation in the same push, the transformation's `variables_id` / `variables_values_id` placeholders are rebound to the assigned ULIDs and the row's `values` are hoisted even without a `_keboola` block, so `job run` succeeds with no post-push `config variables-set` step (unresolvable/ambiguous links add a `variable_link` entry in `errors[]`, never a broken link). Since vNEXT push also remaps shared-code links, flow and orchestrator task `configId`s / `configRowIds` and schedule targets to the configs created in the same push. A link it cannot set is an `errors[]` entry with `change_type` `shared_code_link`, `flow_task_link` or `schedule_target_link` (error code `LINK_UNRESOLVED` for a row id without a new row, `API_ERROR` for a failed PUT). After a failed PUT the local files already hold the new ids and the manifest hash stays stale, so the next `sync push` sends them; this also applies to `variable_link`. The result carries `link_remaps` (`flow_tasks`, `orchestrator_tasks`, `schedule_targets`, `shared_code`, `config_row_ids`); `flow_task_remaps` counts flow tasks only. Never-fetched guard (since v0.72.0): a manifest entry with an empty `pull_hash` and no local files (pre-0.72 name-collision phantom) is **never** planned as a remote DELETE -- diff/push exclude it and report it under `never_fetched` with a warning (run `sync pull` to materialize); local deletion of a properly-pulled config deletes on `push --force`; since vNEXT (#792) a plain push deletes nothing and lists the deletion under `skipped_deletions` (also in `--dry-run`), and a config or row deleted on the remote since the last pull is `remote_deleted`, which push never re-creates. Adopted-by-id writeback (since v0.72.0): pushing an untracked file whose `_keboola.config_id` resolves on the branch also writes the manifest entry, so follow-up diffs are stable. `--branch` (0.47.0+) per-invocation override; when no `/` subtree exists on disk (since 0.47.2) the local default tree (`main/`) is promoted to the target branch (API writes still target the branch id); `--no-name-drift-warnings` (0.47.0+) drops the cosmetic warnings array. Branch-scoped since v0.89.0 (issue #649): push consumes the diff's changeset, so configs tracked on another branch's tree are never planned as creates -- they ride along on the result envelope under `orphaned` instead (see `sync diff`). **Since 0.91.0 (#686)** the manifest baseline `pull_config_hash` is stamped from the API response (or a read-back), not from the files on disk, so a pushed multi-statement SQL transformation -- or anything disabled in the UI whose local YAML lacks `is_disabled` -- no longer shows permanent phantom `REMOTE MODIFIED` drift; if the config cannot be read back after the write the baseline is left UNTOUCHED and a `warnings[]` entry says to run `sync pull` (never a disk-derived fallback). One legacy change is refused per-change with `SYNC_LEGACY_BOUNDARY`: a tree pulled before statement-boundary markers existed whose only difference from the remote is the lost boundaries (pushing it would collapse separate SQL statements into one) -- run `sync pull` for that project first. Ignored components (since 0.91.0) are filtered out on both sides of the diff push builds on, so a stale local directory for an ignored component (e.g. `keboola.mcp-server-tool`) is never classified as `DELETED` and can never be pushed as a remote deletion. +- `sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing] [--with-workspaces]` -- initialize sync working directory; `--adopt-existing` adopts a `.keboola/manifest.json` already written by the kbc Go CLI without overwriting (idempotent; validates `project_id` against the alias token). `--with-workspaces` *(since vNEXT)* sets `syncWorkspaces` in the manifest, so pull/diff/push/clone also sync shared SQL workspaces (`keboola.sandboxes` without `parameters.id`, `runtime.shared: true`), config only; a `push --force` delete removes the workspace's SQL editor sessions first (a plain push holds it back under `skipped_deletions`). With `--adopt-existing` it turns the key on in an existing manifest. See `sync-workflow.md` > "Shared SQL workspaces". +- `sync pull --project ALIAS [--all-projects] [--force] [--theirs] [--dry-run] [--with-samples] [--no-storage] [--no-jobs] [--job-limit N] [--branch ID]` -- download configs to local files. **Auto-inits:** if the target directory has no `.keboola/manifest.json`, pull runs `init` first, so a separate `sync init` is not needed for a first checkout of a project. For large projects (>100 configs), automatically fetches jobs per-config when the grouped API limit is insufficient. `--force` is conflict-aware (since 0.53.0): a locally-modified config whose remote is unchanged is **preserved** (pending delta stays pushable, never silently re-stamped); a true merge conflict (local AND remote both changed since last pull) **aborts** the pull (exit 1, `SYNC_CONFLICT`; `--json` lists `details.conflicts`); local-untouched + remote-changed takes remote. `--theirs` (since v0.72.0) is the supported "discard local, take production" reconcile path: overwrites locally-modified configs/rows, restores deleted/missing files, resolves conflicts by taking remote (no abort, no manifest surgery). Since v0.72.0 plain pull also re-materializes a tracked config whose local dir was deleted (manifest<->disk invariant), so delete-dir-then-pull refetches. Config-level `isDisabled` is kept through pull and push (since v0.72.0) as sparse `is_disabled: true` in `_config.yml` -- absent key = enabled. `--branch` (0.47.0+) per-invocation dev-branch override, beats every other branch source. Ignored components (since 0.91.0): `keboola.sandboxes` + `keboola.mcp-server-tool` are always excluded (`keboola.sandboxes` shared SQL workspaces are synced when the manifest sets `syncWorkspaces`, *since vNEXT*), unioned with the manifest's `ignoredComponents` list; a component newly ignored has its manifest entry dropped and local directory removed, reported with pull action `"ignored"` (distinct from `"removed"` = genuinely deleted on remote). Config-folder round-trip *(since 0.94.0)*: pull captures each config's UI folder (`KBC.configuration.folderName`, from the branch-only `search/component-configurations` endpoint) into the manifest, and reports `folder_lookup_failed` when that lookup fails, keeping the previously captured folder. +- `sync push --project ALIAS [--all-projects] [--dry-run] [--force] [--allow-plaintext-on-encrypt-failure] [--branch ID] [--no-name-drift-warnings]` -- push local changes (auto-encrypts secrets, fails if encryption fails). Workspace delete *(since vNEXT)*: in a `syncWorkspaces` tree, `push --force` deleting a shared SQL workspace also deletes its SQL editor sessions (every user's, push branch) and their backend workspaces, which `config restore` does not bring back, so check `push --dry-run --force` (`warnings[]` `workspace_sessions`) first; `--force` is destructive-class (`sync.push --force`, blocked by `--deny-destructive`). Fresh-CREATE writeback updates placeholder manifest entries in place (since 0.47.0) and propagates any `KBC.configuration.*` metadata via `set_config_metadata`. Fresh-CREATE variable binding (since 0.47.2): when a `keboola.variables` config + its values row are created alongside a transformation in the same push, the transformation's `variables_id` / `variables_values_id` placeholders are rebound to the assigned ULIDs and the row's `values` are hoisted even without a `_keboola` block, so `job run` succeeds with no post-push `config variables-set` step (unresolvable/ambiguous links add a `variable_link` entry in `errors[]`, never a broken link). Since vNEXT push also remaps shared-code links, flow and orchestrator task `configId`s / `configRowIds` and schedule targets to the configs created in the same push. A link it cannot set is an `errors[]` entry with `change_type` `shared_code_link`, `flow_task_link` or `schedule_target_link` (error code `LINK_UNRESOLVED` for a row id without a new row, `API_ERROR` for a failed PUT). After a failed PUT the local files already hold the new ids and the manifest hash stays stale, so the next `sync push` sends them; this also applies to `variable_link`. The result carries `link_remaps` (`flow_tasks`, `orchestrator_tasks`, `schedule_targets`, `shared_code`, `config_row_ids`); `flow_task_remaps` counts flow tasks only. Never-fetched guard (since v0.72.0): a manifest entry with an empty `pull_hash` and no local files (pre-0.72 name-collision phantom) is **never** planned as a remote DELETE -- diff/push exclude it and report it under `never_fetched` with a warning (run `sync pull` to materialize); local deletion of a properly-pulled config deletes on `push --force`; since vNEXT (#792) a plain push deletes nothing and lists the deletion under `skipped_deletions` (also in `--dry-run`), and a config or row deleted on the remote since the last pull is `remote_deleted`, which push never re-creates. Adopted-by-id writeback (since v0.72.0): pushing an untracked file whose `_keboola.config_id` resolves on the branch also writes the manifest entry, so follow-up diffs are stable. `--branch` (0.47.0+) per-invocation override; when no `/` subtree exists on disk (since 0.47.2) the local default tree (`main/`) is promoted to the target branch (API writes still target the branch id); `--no-name-drift-warnings` (0.47.0+) drops the cosmetic warnings array. Branch-scoped since v0.89.0 (issue #649): push consumes the diff's changeset, so configs tracked on another branch's tree are never planned as creates -- they ride along on the result envelope under `orphaned` instead (see `sync diff`). **Since 0.91.0 (#686)** the manifest baseline `pull_config_hash` is stamped from the API response (or a read-back), not from the files on disk, so a pushed multi-statement SQL transformation -- or anything disabled in the UI whose local YAML lacks `is_disabled` -- no longer shows permanent phantom `REMOTE MODIFIED` drift; if the config cannot be read back after the write the baseline is left UNTOUCHED and a `warnings[]` entry says to run `sync pull` (never a disk-derived fallback). One legacy change is refused per-change with `SYNC_LEGACY_BOUNDARY`: a tree pulled before statement-boundary markers existed whose only difference from the remote is the lost boundaries (pushing it would collapse separate SQL statements into one) -- run `sync pull` for that project first. Ignored components (since 0.91.0) are filtered out on both sides of the diff push builds on, so a stale local directory for an ignored component (e.g. `keboola.mcp-server-tool`) is never classified as `DELETED` and can never be pushed as a remote deletion. - `sync clone --source DIR --target ALIAS --target-dir DIR [--bucket-map FILE] [--variable-values FILE] [--instance-rename FILE] [--no-create-buckets] [--dry-run] [--branch ID]` -- clone a reference synced project into a **fresh** target project and parameterize it. Copies the reference tree at `--source` into `--target-dir`, applies declarative overrides from JSON/YAML files (`--bucket-map` `{old_bucket_id: new_bucket_id}` rewrites storage input/output table refs; `--variable-values` `{var_name: value}` overrides `keboola.variables` rows; `--instance-rename` `{old_path_prefix: new_path_prefix}` renames config dirs + manifest paths), re-points the manifest at the target project, and pushes. Because the reference's config ids do not exist in the fresh target, every config is CREATEd fresh and **keboola.flow task `configId`s + transformation variable links are remapped reference->ULID** by push Phase C/D (the push result carries `flow_task_remaps`). Since vNEXT push also remaps shared-code links (`shared_code_id`, `shared_code_row_ids`, `{{}}` script placeholders), legacy `keboola.orchestrator` task `configId`s, task `configRowIds` and a schedule's `target.configurationId`; the clone result carries `link_remaps` per kind (see `sync push`). **`warnings[]` (since vNEXT)**: the clone result (also `--dry-run`; human mode prints them) lists `missing_task_target` (a flow or orchestrator task runs a config that is not in the tree), `encrypted_values_copied` (the `KBC::` paths the target cannot decrypt, as `_config.yml` paths: `secret_keys` get a plaintext + `sync push`, `unencryptable_keys` need `kbagent encrypt values`, `oauth_keys` a new authorization), `data_app_not_deployed` (run `data-app deploy`) and `schedule_not_active` (clone never activates a schedule; `flow schedule` does, and the hint is left out when several schedules run one flow), plus the push warnings. Only the run that creates the configs reports them: a re-run returns `warnings: []`, so keep them from the first run. **Idempotent**: re-running with an existing `--target-dir` skips copy/overrides and just pushes, reporting `no_changes` / `created: 0`. Fails fast (`CONFIG_ERROR`) if the target already contains the reference's configs -- clone requires a fresh/empty target. `SyncService.clone_project(...)` returns a typed `CloneResult` for in-process SDK callers. Override files must be flat `{id: scalar}` mappings *(since v0.89.0)* -- a nested mapping, list, or null value is rejected with `CONFIG_ERROR` (exit 5) naming the key and its actual type. `--branch` is optional on a fresh clone *(since v0.93.1)*. It defaults to the target's production branch, resolved from the API the same way `sync init` does. Pass `--branch ` only to target a dev branch. The config folder (`KBC.configuration.folderName`) is recreated in the target *(since 0.94.0)* -- clone re-points its production configs onto the branch push resolves, so the create-path writeback carries the folder for a plain, `--branch`, and git-branching production clone. **Data-app runtime type (since 0.94.0)**: a `keboola.data-apps` config's type (`python-js` / `streamlit`) lives only on the Data Science `/apps` record, so `sync pull` records it in `_keboola.data_app_type` and clone sends it through `create_app`. A config with no recorded type is created as `python-js`, the default, with a `data_app_type_default` warning *(since vNEXT)*. Re-pull a tree pulled by an older version before cloning a Streamlit app, or it is created as `python-js`. **Storage buckets (since vNEXT)**: clone copies configs, not storage, so a cloned config's input/output mappings point at buckets a fresh target lacks. Clone reads the `storage/buckets.json` pull export and creates the missing buckets in the target **by default** (`--no-create-buckets` skips it) -- idempotent (an existing bucket is skipped, a per-bucket API failure is collected in `bucket_errors`), and the created id is `--bucket-map`-remapped so it matches the rewritten config refs. Each bucket is created on the backend the export recorded. A linked (shared) bucket is linked to the same source as in the reference, under the same id, and listed in `linked_buckets` with its source (an empty bucket in its place would stay empty). The source project's sharing settings decide whether the target may link it; a refused link lands in `bucket_errors`. A tree pulled by an older version does not record which buckets are linked, so clone creates no bucket from it and records one `bucket_errors` entry -- re-pull the reference, or pass `--no-create-buckets`. Only the buckets are created, never their tables or data (the export has no table data) -- populate tables by bucket sharing / `storage upload-table`, or by running the flows. - `sync diff --project ALIAS [--all-projects] [--branch ID]` -- 3-way diff (local vs base vs remote), detects conflicts. `--branch` (0.47.0+) per-invocation dev-branch override. Branch-scoped since v0.89.0 (issue #649): the local side is read from exactly ONE tree (the target branch's subtree, or `main/` when the target has none). Manifest entries belonging to another branch's tree -- what `sync pull --branch ` leaves behind when it re-targets the manifest -- are excluded from the changeset and reported under `orphaned` (`summary.orphaned` + details with `component_id`, `config_id`, `path`, `branch_id`, `branch_path`, `exists_on_target`, `reason`, `hint`); human mode previews the first 10. An orphaned FILE whose `_keboola.config_id` still resolves on the target is adopted (diffed as `unchanged`/`modified`), never re-created; same-tree id claims keep the #482/#497 fork-by-copy CREATE. Fix a non-zero `summary.orphaned` with `sync pull`. **Since 0.91.0 (#686)** a manifest entry without `metadata.config_hash_version` (written by a pre-0.91.0 kbagent) is compared leniently: a stored hash equal to the pre-0.91.0 hash of the SAME remote config counts as in sync, so the phantom `codes changed` entries disappear immediately; every other field is still pinned by that hash, so real remote drift is unaffected. One `sync pull` per project stamps the version and ends the leniency. Ignored components (since 0.91.0) -- `keboola.sandboxes`, `keboola.mcp-server-tool`, and anything listed in the manifest's `ignoredComponents` -- are excluded from BOTH sides of the comparison, so a stale local directory for one of them never shows up as `DELETED`. Since vNEXT (#792) a config or row the manifest fetched from the target branch and that is missing on the remote is `remote_deleted` (human: `- REMOTE DELETED`, `summary.remote_deleted`): run `sync pull`, push never re-creates it. `DELETED` changes are applied only by `sync push --force`. - `sync status [--directory DIR]` -- show locally modified/added/deleted configs. Also surfaces `plaintext_secret_warnings` (since 0.55.0): in-sync configs/rows whose `#`-secrets are still plaintext on the remote (a leftover from pre-0.54.0 writes; #378). Pending (un-pushed) edits are not flagged. Fix = re-push on >=0.54.0 + rotate (version history keeps the plaintext). diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index a626b87b9..390c02a16 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -5523,3 +5523,38 @@ drops manifest entries whose config is gone from the remote) are closed: whose first clone run failed part-way can report the configs it never created as `remote_deleted`. Delete that target directory and run the clone again. + +## `sync` can sync shared SQL workspaces, opt-in per tree (CLI-25) + +*(since vNEXT)* `keboola.sandboxes` is no longer skipped when the manifest sets +`"syncWorkspaces": true` (`sync init --with-workspaces`, or +`sync init --adopt-existing --with-workspaces` for an existing tree). Without +the key nothing changes. Full rules: `sync-workflow.md` > "Shared SQL +workspaces". + +- **Only shared SQL workspaces.** A `keboola.sandboxes` config with no + `parameters.id` and `runtime.shared: true`. Python/R workspaces and legacy + SQL sandboxes carry `parameters.id` and stay skipped. `keboola.mcp-server-tool` + stays ignored. An `ignoredComponents` entry for `keboola.sandboxes` wins. +- **Config only.** Push writes the Storage configuration, never a job, a SQL + editor session or a table load. A `parameters.backendSize` change gets a + `workspace_backend_size` warning: an open session keeps its old size. +- **A delete is not only a config delete.** `push --force` deletes the + workspace's SQL editor sessions (every user's, in the push branch) before the + config, and that drops their backend workspaces, which `config restore` does + not bring back. Check `push --dry-run --force` first: its `workspace_sessions` + warnings list the session ids. If the sessions cannot be listed or deleted, + the config stays. A plain push deletes neither: the workspace is listed under + `skipped_deletions`, like any other deletion, and `skipped_deletions_reason` + says that `--force` also deletes the sessions. +- **`sync clone`** adds a `workspace_input_tables_missing` warning per cloned + workspace whose input tables do not exist in the target (clone creates + buckets, never tables). +- **`sync push --force` is destructive-class** (operation `sync.push --force` + in `permissions list`): a policy denying `cli:destructive`, or + `--deny-destructive`, blocks it, while a plain `sync push` stays write-class. +- **Removing the key** makes the next `sync pull` drop the workspace entries + with action `ignored`, except a workspace edited locally and not pushed: pull + (also `--force`) keeps it and reports it as `skipped`; only `--theirs` + deletes it. A `kbc` manifest save removes the key too (`kbc` writes back only + the keys it knows). diff --git a/plugins/kbagent/skills/kbagent/references/permissions-workflow.md b/plugins/kbagent/skills/kbagent/references/permissions-workflow.md index 78d1615f5..ead9ddcee 100644 --- a/plugins/kbagent/skills/kbagent/references/permissions-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/permissions-workflow.md @@ -77,6 +77,7 @@ The agent can still pull configs and view diffs, but cannot push changes back. N kbagent permissions set --mode allow --deny "cli:destructive" ``` Blocks `branch.delete`, `workspace.delete`, `config.delete`. The agent can still create and modify resources. +*(since vNEXT)* It also blocks `sync push --force` (operation `sync.push --force`, a flag escalation like `auth.logout --remove-projects`): a forced push of a tree that syncs SQL workspaces deletes their SQL editor sessions and workspaces. A plain `sync push` stays write-class and allowed. ### Allow only specific commands (strict allowlist) ```bash diff --git a/plugins/kbagent/skills/kbagent/references/sync-workflow.md b/plugins/kbagent/skills/kbagent/references/sync-workflow.md index 6f4d3d014..360e51527 100644 --- a/plugins/kbagent/skills/kbagent/references/sync-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/sync-workflow.md @@ -459,7 +459,11 @@ internal state: - **Always ignored** -- `keboola.sandboxes` (Workspaces API) and `keboola.mcp-server-tool` (the Keboola MCP server auto-creates one empty workspace-record config per project it touches, `configuration: {}`, name - like `mcp-workspace-`). This is a hardcoded floor; no flag disables it. + like `mcp-workspace-`). This is a hardcoded floor. The one exception + *(since vNEXT)*: the manifest key `syncWorkspaces` takes `keboola.sandboxes` + off the list for its shared SQL workspaces, see + [Shared SQL workspaces](#shared-sql-workspaces). `keboola.mcp-server-tool` + stays ignored. - **Project-configurable** -- the manifest field `ignoredComponents` in `.keboola/manifest.json` adds project-specific exclusions on top of the hardcoded list, without waiting for an upstream kbagent release: @@ -484,6 +488,76 @@ internal state: - **Un-ignoring** a component: remove it from `ignoredComponents` and run `sync pull` again -- it re-materializes like any newly-tracked config. +## Shared SQL workspaces + +*(since vNEXT)* A tree can sync the shared SQL workspaces (Snowflake, +BigQuery) of a project. It is opt-in per tree, with the manifest key +`syncWorkspaces`: + +```bash +kbagent sync init --project prod --with-workspaces # new tree +kbagent sync init --project prod --adopt-existing --with-workspaces # existing tree +``` + +or set `"syncWorkspaces": true` in `.keboola/manifest.json`. Without the key, +sync skips `keboola.sandboxes` exactly as before. + +- **Scope.** Only `keboola.sandboxes` configs WITHOUT `parameters.id` and with + `runtime.shared: true`. A config with `parameters.id` is a Python/R + (container) workspace, or a legacy SQL sandbox from before the SQL editor; + both stay skipped. The official CLI tells SQL from Python/R by the same key. + A non-shared workspace is visible only to its creator in the UI, so pull + does not fetch it. A workspace already tracked stays tracked when someone + turns `runtime.shared` off: diff reports the change as `remote_modified`. +- **Pull / diff.** A normal `_config.yml`: `parameters.blocks` (the SQL + scripts), `parameters.backendSize`, `input` / `output` (including + `read_only_storage_access` when it is off), and under + `_configuration_extra` the `runtime.shared` flag and the + `shared_code_id` / `shared_code_row_ids` / `variables_id` / + `variables_values_id` links of a workspace created from a transformation. + The config holds no credentials. +- **Push create / update** writes the Storage configuration only: no Queue + job, no SQL editor session, no table load. The UI creates the session when + a user opens the workspace. When `parameters.backendSize` changes, push adds + a `workspace_backend_size` warning: an existing session keeps its size, the + new size applies only to a session created later. Dev branches work the + same way (config only). +- **Push delete** needs `--force`, like every delete: a plain push lists the + workspace under `skipped_deletions` and touches neither its sessions nor its + configuration; `skipped_deletions_reason` then says what `--force` also + deletes. `push --force` first deletes the workspace's SQL editor + sessions in the push branch, of every user, then the configuration, like + `kbc remote workspace delete`. Deleting a session also drops its backend + workspace, which a config restore does not bring back. When the sessions + cannot be listed, or one cannot be deleted (for example it is still + initializing), push keeps the configuration and reports the error. The + deleted session ids are in `pushed_details[].deleted_session_ids`. + `push --dry-run --force` adds one `workspace_sessions` warning per deleted + workspace with `session_count` and `session_ids`; a plain `push --dry-run` + previews no session delete. A tracked workspace whose + config now has `parameters.id` (backed by a Data Science app) is not + deleted: push reports a `VALIDATION_ERROR`, and `push --dry-run --force` + reports `workspace_delete_refused` for it instead of a session list. When + the config delete fails after the sessions were deleted, the error names + those sessions. `sync push --force` needs the + destructive permission class (`--deny-destructive` blocks it). +- **Clone** creates the workspace configs like other configs; `bucket_map` + rewrites their input mapping. Clone creates buckets, never tables, so the + clone result carries one `workspace_input_tables_missing` warning per + workspace whose input tables do not exist in the target. +- **Turning it off.** Remove the key: the next `sync pull` drops the workspace + entries and their directories with action `"ignored"`. A workspace edited + locally and not pushed is kept instead (entry and directory), reported with + action `"skipped"` and the reason; plain pull and `--force` both keep it, + only `--theirs` deletes it. Diff and push ignore the kept entry, so set the + key again and push to apply the edit. An `ignoredComponents` entry for + `keboola.sandboxes` wins over the key. +- **Mixed trees.** `kbc` reads the manifest with unknown keys ignored, so the + key does not break it. A manifest save by `kbc` writes only the keys `kbc` + knows, so it removes `syncWorkspaces`. `kbc` itself always ignores + `keboola.sandboxes`: it logs a warning for each workspace entry in the + manifest and skips it. + ## `sync pull --force` is conflict-aware (since 0.53.0) `--force` no longer blindly overwrites locally-modified configs. It branches on diff --git a/src/keboola_agent_cli/client/_client.py b/src/keboola_agent_cli/client/_client.py index 45ee77676..38b99e5c9 100644 --- a/src/keboola_agent_cli/client/_client.py +++ b/src/keboola_agent_cli/client/_client.py @@ -2,7 +2,7 @@ ``KeboolaClient`` is assembled here from the per-family mixins (storage tables, storage files, configs, queue, tokens, branches, merge requests, stream, -query, workspaces, billing, notifications, misc) over the shared +query, workspaces, billing, notifications, editor, misc) over the shared ``_CoreClient`` plumbing base. It stays a single class exposing every Storage/Queue method at its original signature, so ``keboola_agent_cli.Client`` and its ``.raw`` accessor are unaffected by the split of the former single-file ``client.py`` into a @@ -17,6 +17,7 @@ from .billing import _BillingMixin from .branches import _BranchesMixin from .configs import _ConfigsMixin +from .editor import _EditorMixin from .merge_requests import _MergeRequestsMixin from .misc import _MiscMixin from .notifications import _NotificationsMixin @@ -43,6 +44,7 @@ class KeboolaClient( _WorkspacesMixin, _BillingMixin, _NotificationsMixin, + _EditorMixin, _TriggersMixin, _MiscMixin, _CoreClient, diff --git a/src/keboola_agent_cli/client/_core.py b/src/keboola_agent_cli/client/_core.py index ba5f464a9..83b539cd3 100644 --- a/src/keboola_agent_cli/client/_core.py +++ b/src/keboola_agent_cli/client/_core.py @@ -73,6 +73,7 @@ def __init__(self, stack_url: str, token: str, *, http_auth: httpx.Auth | None = self._sync_actions_client: httpx.Client | None = None self._billing_client: httpx.Client | None = None self._notification_client: httpx.Client | None = None + self._editor_client: httpx.Client | None = None # Lazily built on first Data Streams call (per-device OTLP sources); the # Stream control plane is a sibling host reachable from this stack+token. self._stream_client: StreamClient | None = None @@ -107,6 +108,10 @@ def _billing_base_url(self) -> str: def _notification_base_url(self) -> str: return self._derive_service_url(self._stack_url, "notification") + @property + def _editor_base_url(self) -> str: + return self._derive_service_url(self._stack_url, "editor") + def close(self) -> None: """Close the underlying HTTP clients.""" super().close() @@ -122,6 +127,8 @@ def close(self) -> None: self._billing_client.close() if self._notification_client is not None: self._notification_client.close() + if self._editor_client is not None: + self._editor_client.close() if self._stream_client is not None: self._stream_client.close() @@ -229,6 +236,20 @@ def _notification_request(self, method: str, path: str, **kwargs: Any) -> httpx. method, path, client=client, base_url=self._notification_base_url, **kwargs ) + def _editor_request(self, method: str, path: str, **kwargs: Any) -> httpx.Response: + """Execute an Editor Service (SQL editor sessions) request with retry. + + The editor service is a sibling host derived from the stack URL + (``editor.{stack-suffix}``, the ``editor`` entry of ``GET /v2/storage``); + the sub-client inherits the main client's headers, so the + ``X-StorageApi-Token`` auth carries over. The service also accepts a + bearer token, which ``_get_or_create_sub_client`` passes on. + """ + client = self._get_or_create_sub_client("_editor_client", self._editor_base_url) + return self._do_request( + method, path, client=client, base_url=self._editor_base_url, **kwargs + ) + def _billing_get(self, path: str, **kwargs: Any) -> httpx.Response: """Execute a read-only Billing API request with retry. diff --git a/src/keboola_agent_cli/client/editor.py b/src/keboola_agent_cli/client/editor.py new file mode 100644 index 000000000..e736f21ab --- /dev/null +++ b/src/keboola_agent_cli/client/editor.py @@ -0,0 +1,73 @@ +"""Editor Service: SQL editor sessions of ``keboola.sandboxes`` workspaces (CLI-25). + +A SQL workspace (Snowflake / BigQuery) created in the UI is only a +``keboola.sandboxes`` configuration. Its backend workspace belongs to a SQL +editor *session*, which the editor service creates when a user first opens +the workspace. ``sync push`` deletes a workspace the way the official CLI does +(``kbc remote workspace delete``): the sessions first, then the configuration. +Deleting a session also drops its Storage workspace (asynchronously, on the +service side). + +Request and response shapes, from the service's OpenAPI spec (keboola/editor-service +``docs/swagger.yaml``) and keboola-sdk-go ``pkg/keboola/editor_session.go``: + +- ``GET /sql/sessions`` lists the sessions of the CURRENT user only. + ``listAll=1`` lists every session in the project; ``branchId`` limits the + list to one branch. There is no filter by configuration. +- ``DELETE /sql/sessions/{id}`` answers 204. The service refuses (4xx) a + session that is still ``initializing``. +- A session carries ``snowflakePrivateKey`` only when ``includeCredentials`` + is sent. This mixin never sends it. + +The session list fails closed: a body that is not a JSON array of objects +raises instead of reading as "no sessions", because the caller would then +delete the workspace config and leave its sessions and workspaces behind. +""" + +from typing import Any +from urllib.parse import quote + +from ..errors import ErrorCode, KeboolaApiError +from ._core import _CoreClient + + +class _EditorMixin(_CoreClient): + """Editor Service: list and delete SQL editor sessions.""" + + def list_editor_sessions(self, branch_id: int | None = None) -> list[dict[str, Any]]: + """List the SQL editor sessions of every user in the project. + + Args: + branch_id: If set, only the sessions of this branch (sent as + ``branchId``). The production branch is its numeric id too. + + Returns: + List of session dicts as the API returns them. + + Raises: + KeboolaApiError: ``API_ERROR`` when the body is not valid JSON or + not an array of objects (see the module docstring). + """ + params: dict[str, str] = {"listAll": "1"} + if branch_id is not None: + params["branchId"] = str(branch_id) + response = self._editor_request("GET", "/sql/sessions", params=params) + try: + body = response.json() + except ValueError: + body = None + if not isinstance(body, list) or not all(isinstance(item, dict) for item in body): + raise KeboolaApiError( + message=( + "Editor service returned an unexpected SQL editor session list " + f"(HTTP {response.status_code}, not a JSON array of sessions)." + ), + status_code=response.status_code, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + return body + + def delete_editor_session(self, session_id: str) -> None: + """Delete one SQL editor session (and, service-side, its workspace).""" + self._editor_request("DELETE", f"/sql/sessions/{quote(str(session_id), safe='')}") diff --git a/src/keboola_agent_cli/commands/_sync_push_render.py b/src/keboola_agent_cli/commands/_sync_push_render.py index 89d27d4ce..f1bcd7b1e 100644 --- a/src/keboola_agent_cli/commands/_sync_push_render.py +++ b/src/keboola_agent_cli/commands/_sync_push_render.py @@ -3,13 +3,17 @@ Split out of ``commands/sync.py``, which is at its size ceiling. Push lists what it did not apply in every result: remote-side changes that need a ``sync pull`` (``skipped``) and the deletions it held back because -``--force`` was not given (``skipped_deletions``). +``--force`` was not given (``skipped_deletions``). The same notice prints the +result's ``warnings[]`` (e.g. the SQL editor sessions a workspace delete +removes, CLI-25). """ from __future__ import annotations from typing import Any +from rich.markup import escape + # Remote-side change types in the diff, labelled for human output. REMOTE_CHANGE_LABELS = { "remote_modified": "~ REMOTE MODIFIED", @@ -18,10 +22,14 @@ def print_push_skips(formatter: Any, result: dict[str, Any]) -> None: - """Print the changes a push result did not apply, with the next step. + """Print the changes a push result did not apply, with the next step, and its warnings. - A notice, not a result, so it goes to stderr like every other hint. + A notice, not a result, so it goes to stderr like every other hint. A + warning message quotes config names and paths, which can hold ``[...]``; + it is escaped so Rich prints it as text instead of reading it as markup. """ + for warn in result.get("warnings", []): + formatter.warning(f" {escape(str(warn.get('message', '')))}") skipped_reason = result.get("skipped_reason") if skipped_reason: formatter.err_console.print(f" [yellow]{skipped_reason}[/yellow]") diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 967230f73..411b67286 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -1545,8 +1545,23 @@ ### Project Sync - kbagent sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing] + kbagent sync init --project ALIAS [--directory DIR] [--git-branching] [--adopt-existing] [--with-workspaces] Initialize sync working directory. --git-branching enables git-to-Keboola branch mapping. + --with-workspaces (since vNEXT, CLI-25) sets "syncWorkspaces": true in the manifest + (with --adopt-existing: turns it on in an existing one). pull/diff/push/clone then also + sync shared SQL workspaces: keboola.sandboxes configs with no parameters.id and + runtime.shared true (Python/R and legacy SQL sandboxes carry parameters.id and stay + skipped). Config only: push never runs a job, opens a SQL editor session or loads + tables; a parameters.backendSize change adds a workspace_backend_size warning (an open + session keeps its size). A `push --force` DELETE removes the workspace's SQL editor + sessions of every user in the push branch, then the config (a plain push holds it back + under skipped_deletions); if listing/deleting sessions fails the config stays and the + error is reported. push --dry-run --force lists them (warnings[] workspace_sessions: + session_count, session_ids). sync clone warns per workspace whose + input tables the target lacks (workspace_input_tables_missing). Removing the key makes + the next pull drop the entries as "ignored", except a locally edited workspace, which + pull (also --force) keeps and reports as "skipped" (only --theirs deletes it). An + ignoredComponents entry wins. kbagent sync pull --project ALIAS [--all-projects] [--force] [--theirs] [--dry-run] [--with-samples] [--no-storage] [--no-jobs] [--job-limit N] [--branch ID] Download configs as local files. Idempotent, protects local modifications. @@ -1567,7 +1582,8 @@ Auto-detects renamed configs and renames local directories to match (uses git mv in git repos). --branch: per-invocation dev-branch override. Same semantics as sync push/diff. Ignored components (since 0.91.0, #689): keboola.sandboxes + keboola.mcp-server-tool are - always excluded, unioned with the manifest's ignoredComponents list + always excluded (except shared SQL workspaces under syncWorkspaces, since vNEXT), + unioned with the manifest's ignoredComponents list (.keboola/manifest.json) -- a per-tree exclusion knob honored by pull/diff/push. A component newly ignored has its manifest entry dropped and local dir removed on the next pull, reported with details[].action "ignored" -- distinct from "removed", which means @@ -1606,6 +1622,11 @@ kbagent sync push --project ALIAS [--all-projects] [--dry-run] [--force] [--allow-plaintext-on-encrypt-failure] [--branch ID] [--no-name-drift-warnings] Push local changes. Auto-encrypts secrets. Skips conflicts (pull first). Fails if encryption fails (plaintext secrets never pushed). Use escape hatch flag only if you know what you are doing. + Workspace delete (since vNEXT, syncWorkspaces trees): a --force push that deletes a shared + SQL workspace also deletes its SQL editor sessions (every user's, push branch) and their + backend workspaces, which config restore does not bring back; check + `sync push --dry-run --force` (warnings[] workspace_sessions) first. --force is destructive-class (a policy denying + cli:destructive or --deny-destructive blocks it; plain push stays write-class). Fresh-CREATE behavior: if the manifest contains a placeholder entry at (component_id, path), the create path updates it in place (no manifest duplication) and propagates any KBC.configuration.* metadata via set_config_metadata. Re-pushes diff --git a/src/keboola_agent_cli/commands/sync.py b/src/keboola_agent_cli/commands/sync.py index b61f6fdb0..71292bb5d 100644 --- a/src/keboola_agent_cli/commands/sync.py +++ b/src/keboola_agent_cli/commands/sync.py @@ -11,7 +11,13 @@ from ..constants import SYNC_ORPHAN_PREVIEW_LIMIT from ..errors import ConfigError, ErrorCode, KeboolaApiError, SyncConflictError -from ._helpers import check_cli_permission, get_formatter, get_service, map_error_to_exit_code +from ._helpers import ( + check_cli_operation, + check_cli_permission, + get_formatter, + get_service, + map_error_to_exit_code, +) from ._sync_clone_render import print_clone_result from ._sync_push_render import REMOTE_CHANGE_LABELS, print_push_skips, push_skips_one_liner @@ -103,6 +109,9 @@ def sync_init( "instead of failing. Validates the manifest's project_id against the alias " "and normalises the file. Idempotent.", ), + with_workspaces: bool = typer.Option( + False, "--with-workspaces", help="Also sync shared SQL workspaces (keboola.sandboxes)." + ), ) -> None: """Initialize a sync working directory for a Keboola project. @@ -112,6 +121,10 @@ def sync_init( Use --adopt-existing to register a directory that was already initialised by the official kbc CLI without overwriting the manifest. + + Use --with-workspaces to set syncWorkspaces in the manifest: pull, diff, + push and clone then also handle shared SQL workspaces. With --adopt-existing + it turns the key on in an existing manifest. """ formatter = get_formatter(ctx) service = get_service(ctx, "sync_service") @@ -123,6 +136,7 @@ def sync_init( project_root=project_root, git_branching=git_branching, adopt_existing=adopt_existing, + sync_workspaces=with_workspaces, ) except ConfigError as exc: formatter.error(message=exc.message, error_code=ErrorCode.CONFIG_ERROR) @@ -991,8 +1005,9 @@ def sync_push( False, "--force", help=( - "Delete remote configs and rows whose local files were removed. " - "Without it push skips those deletions and lists them." + "Delete remote configs and rows whose local files were removed (for a SQL " + "workspace also its SQL editor sessions). Without it push skips those deletions " + "and lists them." ), ), allow_plaintext: bool = typer.Option( @@ -1023,9 +1038,14 @@ def sync_push( Use --project for a single project or --all-projects for all configured projects in parallel. + + --force needs the destructive permission class: a forced delete of a SQL + workspace also deletes its SQL editor sessions and their workspaces. """ formatter = get_formatter(ctx) service = get_service(ctx, "sync_service") + if force: + check_cli_operation(ctx, "sync.push --force") if all_projects and project: formatter.error( @@ -1131,8 +1151,6 @@ def sync_push( f" Error: {err['change_type']} {err['component_id']}/{err['config_id']}: " f"{err['message']}" ) - for warn in result.get("warnings", []): - formatter.warning(f" {warn['message']}") @sync_app.command("clone") diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index 750ec3c9d..f970e97c9 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -760,7 +760,7 @@ def _resolve_app_name() -> str: # Components that are always excluded from sync operations (pull/push/diff). # These are managed through separate APIs and have volatile internal state. # A project may extend this set per working tree via the manifest's -# ``ignoredComponents`` field -- see ``SyncService._effective_ignored_components``. +# ``ignoredComponents`` field -- see ``_sync_workspace.effective_ignored_components``. ALWAYS_IGNORED_COMPONENTS: frozenset[str] = frozenset( { "keboola.sandboxes", # Workspaces API; parameters.id is volatile diff --git a/src/keboola_agent_cli/permissions.py b/src/keboola_agent_cli/permissions.py index 92084c63c..489685269 100644 --- a/src/keboola_agent_cli/permissions.py +++ b/src/keboola_agent_cli/permissions.py @@ -432,8 +432,14 @@ # `project remove`. Without this, a policy denying `cli:admin` to keep an agent # out of the project registry would still let it de-register projects through # `auth`. +# +# `sync push --force` applies the deletions push plans. For a SQL workspace +# (CLI-25) that also deletes its SQL editor sessions and their workspaces, +# which a config restore does not bring back, so it is destructive while a +# plain push is only a write. FLAG_ESCALATIONS: dict[str, str] = { "auth.logout --remove-projects": "admin", + "sync.push --force": "destructive", } # Operations that exist ONLY on the `kbagent serve` REST surface. They are real diff --git a/src/keboola_agent_cli/services/_sync_baseline.py b/src/keboola_agent_cli/services/_sync_baseline.py index 520101d40..7c231900d 100644 --- a/src/keboola_agent_cli/services/_sync_baseline.py +++ b/src/keboola_agent_cli/services/_sync_baseline.py @@ -482,7 +482,7 @@ def detect_force_pull_conflicts( changed. ``ignored_components`` is the caller's pre-computed effective set - (``SyncService._effective_ignored_components``: the hardcoded always-ignored + (``_sync_workspace.effective_ignored_components``: the hardcoded always-ignored components plus the manifest's ``ignoredComponents``, issue #689) so this guard can never disagree with pull/diff about what is ignored. """ diff --git a/src/keboola_agent_cli/services/_sync_clone_warnings.py b/src/keboola_agent_cli/services/_sync_clone_warnings.py index da27a435d..1e6b8b298 100644 --- a/src/keboola_agent_cli/services/_sync_clone_warnings.py +++ b/src/keboola_agent_cli/services/_sync_clone_warnings.py @@ -9,7 +9,9 @@ - a flow / orchestration task that runs a config which is not in the tree, - encrypted (``KBC::``) values, which only the reference project can decrypt, - a data app, which sync creates but never deploys, -- a schedule, which sync never registers with the Scheduler service. +- a schedule, which sync never registers with the Scheduler service, +- a SQL workspace whose input tables do not exist in the target (CLI-25, + built in ``_sync_workspace``). Only the run that creates the configs reports them: a re-run that creates nothing returns no warnings, so the caller must keep them. @@ -26,6 +28,12 @@ from ._encryption import find_encrypted_secret_paths, find_unencryptable_secret_paths from ._sync_bindings import task_config_ref from ._sync_models import FLOW_COMPONENT_ID, ORCHESTRATOR_COMPONENT_ID, SCHEDULER_COMPONENT_ID +from ._sync_workspace import ( + SANDBOXES_COMPONENT_ID, + ClonedWorkspace, + cloned_workspace, + missing_input_table_warnings, +) from .data_app_service import DATA_APP_COMPONENT_ID if TYPE_CHECKING: @@ -219,6 +227,7 @@ def collect_clone_warnings( ) warnings: list[dict[str, Any]] = [] + workspaces: list[ClonedWorkspace] = [] for change in added: component_id = change["component_id"] path = change.get("path", "") @@ -243,6 +252,11 @@ def collect_clone_warnings( warnings.append(_data_app_warning(config, context)) if component_id == SCHEDULER_COMPONENT_ID: warnings.append(_schedule_warning(config, context)) + if component_id == SANDBOXES_COMPONENT_ID: + workspace = cloned_workspace(config.config_id, path, local_data) + if workspace is not None: + workspaces.append(workspace) + warnings.extend(missing_input_table_warnings(service, target_alias, workspaces)) return warnings diff --git a/src/keboola_agent_cli/services/_sync_push_ops.py b/src/keboola_agent_cli/services/_sync_push_ops.py index 90632b2c0..26cdbce8d 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -26,6 +26,7 @@ from ._encryption import encrypt_secrets_in_config from ._sync_baseline import apply_stamp, row_baseline from ._sync_data_app import create_synced_data_app +from ._sync_workspace import SANDBOXES_COMPONENT_ID, warn_on_backend_size_change from ._sync_writeback import writeback_after_push, writeback_create_row_in_manifest from .data_app_service import DATA_APP_COMPONENT_ID, DEFAULT_TYPE @@ -42,6 +43,12 @@ "Push deletes remote configs and rows only with --force. " "Run 'kbagent sync push --force' to delete these." ) +# Added to the reason when a held-back deletion is a SQL workspace (CLI-25). +SKIPPED_WORKSPACE_DELETIONS_NOTE = ( + " For a SQL workspace (keboola.sandboxes), --force also deletes the SQL editor sessions " + "of all users with their Snowflake/BigQuery workspaces, which cannot be restored. " + "Run 'kbagent sync push --dry-run --force' first to list them." +) @dataclass @@ -73,6 +80,11 @@ def report(self) -> dict[str, Any]: if self.skipped_deletions: report["skipped_deletions"] = self.skipped_deletions report["skipped_deletions_reason"] = SKIPPED_DELETIONS_REASON + if any( + change["component_id"] == SANDBOXES_COMPONENT_ID and not change.get("is_row") + for change in self.skipped_deletions + ): + report["skipped_deletions_reason"] += SKIPPED_WORKSPACE_DELETIONS_NOTE return report @@ -585,7 +597,8 @@ def push_update( Returns the API response so the caller can stamp the manifest baseline from the remote's own view of the config (issue #686). ``warnings`` accumulates the ``script[]`` normalization records of - :func:`guard_script_shape` for the push envelope. + :func:`guard_script_shape` for the push envelope, and for a workspace a + ``parameters.backendSize`` change (CLI-25). """ branch_path = service._resolve_source_branch_path(manifest, project_root, branch_id) config_dir = project_root / branch_path / config_path_str @@ -617,6 +630,15 @@ def push_update( allow_plaintext_fallback=allow_plaintext_fallback, ) + if component_id == SANDBOXES_COMPONENT_ID and warnings is not None: + warn_on_backend_size_change( + client, + config_id=config_id, + configuration=configuration, + branch_id=branch_id, + warnings=warnings, + ) + result = client.update_config( component_id=component_id, config_id=config_id, diff --git a/src/keboola_agent_cli/services/_sync_stale.py b/src/keboola_agent_cli/services/_sync_stale.py index 32dfa0320..6e44270c8 100644 --- a/src/keboola_agent_cli/services/_sync_stale.py +++ b/src/keboola_agent_cli/services/_sync_stale.py @@ -15,6 +15,11 @@ its remote vanished. Now plain pull preserves it (entry kept, reported as ``skipped``), ``--force`` aborts with SYNC_CONFLICT, and only ``--theirs`` (remote wins) still deletes it. + +An ``ignored`` entry is deleted even when edited, except a shared SQL +workspace (CLI-25): turning ``syncWorkspaces`` off must not delete SQL typed +into its ``_config.yml`` and never pushed. Plain pull and ``--force`` keep such +an entry and report it as ``skipped``; only ``--theirs`` deletes it. """ from __future__ import annotations @@ -28,6 +33,7 @@ from ..constants import CONFIG_FILENAME from ..sync.manifest import ManifestConfigRow, ManifestConfiguration from ._sync_baseline import extras_modified +from ._sync_workspace import SANDBOXES_COMPONENT_ID if TYPE_CHECKING: from .sync_service import SyncService @@ -35,6 +41,9 @@ logger = logging.getLogger(__name__) REMOTE_DELETED_REASON = "locally modified, deleted on remote" +WORKSPACE_SYNC_OFF_REASON = ( + "locally modified, workspace sync is off; set syncWorkspaces and push, or pull --theirs" +) @dataclass @@ -45,6 +54,11 @@ class StaleEntry: action: str # "removed" (gone from the remote) or "ignored" (component ignored) locally_modified: bool + @property + def keep_reason(self) -> str: + """The ``skipped`` reason pull reports when it keeps this edited entry.""" + return REMOTE_DELETED_REASON if self.action == "removed" else WORKSPACE_SYNC_OFF_REASON + def _entry_locally_modified( service: SyncService, config_dir: Path, entry: ManifestConfiguration @@ -90,9 +104,8 @@ def find_stale_entries( if f"{entry.component_id}/{entry.id}" in remote_keys: continue action = "ignored" if entry.component_id in ignored_components else "removed" - modified = action == "removed" and _entry_locally_modified( - service, branch_dir / entry.path, entry - ) + guarded = action == "removed" or entry.component_id == SANDBOXES_COMPONENT_ID + modified = guarded and _entry_locally_modified(service, branch_dir / entry.path, entry) stale.append(StaleEntry(entry=entry, action=action, locally_modified=modified)) return stale @@ -103,7 +116,11 @@ def reserved_paths(stale: list[StaleEntry], branch_dir: Path) -> set[str]: def remote_deleted_conflicts(stale: list[StaleEntry]) -> list[dict[str, str]]: - """``--force`` conflicts: locally edited configs whose remote was deleted (C).""" + """``--force`` conflicts: locally edited configs whose remote was deleted (C). + + An edited ignored workspace is no conflict (its remote still exists): it is + kept like on a plain pull. + """ return [ { "scope": "config", @@ -114,7 +131,7 @@ def remote_deleted_conflicts(stale: list[StaleEntry]) -> list[dict[str, str]]: "reason": "deleted on remote", } for s in stale - if s.locally_modified + if s.locally_modified and s.action == "removed" ] @@ -158,7 +175,7 @@ def apply_stale_sweep( "component_id": s.entry.component_id, "config_name": s.entry.path, "path": s.entry.path, - "reason": REMOTE_DELETED_REASON, + "reason": s.keep_reason, } ) continue diff --git a/src/keboola_agent_cli/services/_sync_workspace.py b/src/keboola_agent_cli/services/_sync_workspace.py new file mode 100644 index 000000000..1d61a013f --- /dev/null +++ b/src/keboola_agent_cli/services/_sync_workspace.py @@ -0,0 +1,461 @@ +"""Shared SQL workspaces in the sync engine (CLI-25). + +``keboola.sandboxes`` is on :data:`ALWAYS_IGNORED_COMPONENTS`. A tree opts in +with the manifest key ``syncWorkspaces`` (``sync init --with-workspaces``); +then pull, diff, push and clone handle its **shared SQL workspaces** like any +other configuration. Scope and rules: + +- **Which workspaces.** A Snowflake / BigQuery workspace created in the UI is a + ``keboola.sandboxes`` configuration WITHOUT ``parameters.id``: its backend + workspace belongs to a SQL editor session, not to the config. A Python / R + (container) workspace carries ``parameters.id``, the id of its Data Science + ``/apps`` record. The official CLI tells the two apart by the same key + (keboola-as-code ``remote workspace detail``). A legacy SQL sandbox from + before the SQL editor also carries ``parameters.id`` (an ``/apps`` record of + type ``snowflake`` / ``bigquery``); it stays skipped, because its workspace + belongs to that record and a config-only sync cannot create or delete it. + Only ``runtime.shared: true`` workspaces are fetched: the UI shows the + others to their creator only. +- **Tracked stays tracked.** A workspace already in the manifest stays in the + remote listing even when someone turns ``runtime.shared`` off. Diff then + reports the change and pull writes it, instead of the entry vanishing from + the listing, which diff would read as a local ``added`` and push would + create again. +- **Config only.** Push creates and updates the Storage configuration, nothing + else: no Queue job, no SQL editor session, no table load. The UI creates + the session when a user opens the workspace. +- **Delete.** Only ``push --force`` deletes (``plan_push``; a plain push + lists the workspace under ``skipped_deletions``). It deletes the SQL editor + sessions of the workspace (every user's, in the push branch) and then the + configuration, like + ``kbc remote workspace delete``. When the sessions cannot be listed or one + cannot be deleted, the configuration is not deleted and the error names the + sessions already deleted. A workspace whose config has ``parameters.id`` is + refused: its Data Science app would keep running. +- **Turning it off.** Removing the key drops the workspace entries on the + next pull (``_sync_stale``), except an edited one, which is kept. + +Kept out of ``sync_service`` (frozen at its size budget) like the data-app +helpers in ``_sync_data_app``. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING, Any + +from ..constants import ALWAYS_IGNORED_COMPONENTS +from ..errors import ErrorCode, KeboolaApiError +from ..sync.manifest import Manifest, load_manifest +from .base import find_default_branch_id + +if TYPE_CHECKING: + from .sync_service import SyncService + +logger = logging.getLogger(__name__) + +SANDBOXES_COMPONENT_ID = "keboola.sandboxes" + + +def effective_ignored_components(manifest: Manifest) -> frozenset[str]: + """Components excluded from this working tree's sync operations. + + The hardcoded :data:`ALWAYS_IGNORED_COMPONENTS` plus the manifest's + ``ignoredComponents``, which was declared in the schema from day one but + read by nothing until issue #689. ``keboola.sandboxes`` leaves the + hardcoded set when the manifest sets ``syncWorkspaces`` (CLI-25); + ``ignoredComponents`` is added after that, so an explicit + ``keboola.sandboxes`` entry there still wins over the opt-in. + + Computed ONCE per pull/diff and threaded through every filtering site so + the remote side, the local side and the force-pull conflict guard can + never disagree about what is ignored -- a disagreement is what turns a + tracked-but-unfetchable config into a phantom "added" that ``sync push`` + duplicates on the remote, once per push. + """ + always = ALWAYS_IGNORED_COMPONENTS + if manifest.sync_workspaces: + always = always - {SANDBOXES_COMPONENT_ID} + return always | frozenset(manifest.ignored_components) + + +def is_shared_sql_workspace(config: dict[str, Any]) -> bool: + """True for a ``keboola.sandboxes`` API config that sync fetches. + + A SQL workspace has no ``parameters.id`` (missing or empty, as the UI + reads it); ``runtime.shared`` must be true. See the module docstring. + """ + body = config.get("configuration") or {} + parameters = body.get("parameters") or {} + runtime = body.get("runtime") or {} + if not isinstance(parameters, dict) or not isinstance(runtime, dict): + return False + return not parameters.get("id") and runtime.get("shared") is True + + +def scope_listing(components: list[dict[str, Any]], manifest: Manifest) -> list[dict[str, Any]]: + """Drop the out-of-scope ``keboola.sandboxes`` configs from a remote listing. + + Applied right after ``list_components_with_configs`` in pull AND diff, so + every later filtering site (the fetch loop, the stale sweep, the force-pull + conflict guard, the remote side of the diff) sees the same set. Returns + *components* unchanged when the tree has not opted in: the ignored set + already drops the whole component then. + """ + if not manifest.sync_workspaces: + return components + tracked = {c.id for c in manifest.configurations if c.component_id == SANDBOXES_COMPONENT_ID} + scoped: list[dict[str, Any]] = [] + for component in components: + if component.get("id") != SANDBOXES_COMPONENT_ID: + scoped.append(component) + continue + configs = [ + cfg + for cfg in component.get("configurations", []) + if str(cfg.get("id", "")) in tracked or is_shared_sql_workspace(cfg) + ] + scoped.append({**component, "configurations": configs}) + return scoped + + +def _backend_size(configuration: Any) -> Any: + parameters = configuration.get("parameters") if isinstance(configuration, dict) else None + return parameters.get("backendSize") if isinstance(parameters, dict) else None + + +def warn_on_backend_size_change( + client: Any, + *, + config_id: str, + configuration: dict[str, Any], + branch_id: int | None, + warnings: list[dict[str, Any]], +) -> None: + """Warn when a push changes a workspace's ``parameters.backendSize``. + + The editor service reads the size only when it creates a session; an + existing session keeps the size it was created with. Compares with the + remote config just before the update. Best-effort: a failed read logs and + skips the warning, it never blocks the push. + """ + try: + remote = client.get_config_detail( + component_id=SANDBOXES_COMPONENT_ID, config_id=config_id, branch_id=branch_id + ) + except KeboolaApiError as exc: + logger.warning("backendSize check skipped for workspace %s: %s", config_id, exc.message) + return + old_size = _backend_size(remote.get("configuration") if isinstance(remote, dict) else None) + new_size = _backend_size(configuration) + if old_size == new_size: + return + warnings.append( + { + "change_type": "workspace_backend_size", + "component_id": SANDBOXES_COMPONENT_ID, + "config_id": config_id, + "old_backend_size": old_size, + "new_backend_size": new_size, + "message": ( + f"Workspace {config_id}: parameters.backendSize changed from {old_size!r} to " + f"{new_size!r}. An existing SQL editor session keeps its size; the new size " + "applies only to a session created after this push." + ), + } + ) + + +def _session_branch_id(client: Any, branch_id: int | None) -> int: + """The branch id SQL editor sessions carry; production is its numeric id.""" + if branch_id is not None: + return branch_id + default_branch_id = find_default_branch_id(client.list_dev_branches()) + if default_branch_id is None: + raise KeboolaApiError( + message="Cannot find the default branch, so its SQL editor sessions cannot be listed.", + status_code=0, + error_code=ErrorCode.API_ERROR, + retryable=False, + ) + return default_branch_id + + +def list_workspace_sessions( + client: Any, config_ids: set[str], branch_id: int | None +) -> dict[str, list[str]]: + """Map each workspace config id to the ids of its SQL editor sessions. + + Lists every user's sessions (``listAll=1``) of the push branch once. The + branch is checked again here: config ids are the same in every branch, so + a production delete must not reach a dev branch's sessions. + + Raises: + KeboolaApiError: the branch or the sessions cannot be read. + """ + session_branch_id = _session_branch_id(client, branch_id) + sessions: dict[str, list[str]] = {config_id: [] for config_id in config_ids} + for session in client.list_editor_sessions(branch_id=session_branch_id): + config_id = str(session.get("configurationId", "")) + if ( + config_id in sessions + and session.get("componentId") == SANDBOXES_COMPONENT_ID + and str(session.get("branchId", "")) == str(session_branch_id) + and session.get("id") + ): + sessions[config_id].append(str(session["id"])) + return sessions + + +def _app_id(client: Any, config_id: str, branch_id: int | None) -> str | None: + """The ``parameters.id`` of the remote workspace config, or ``None``. + + A ``parameters.id`` means a Python/R or legacy SQL workspace: its backend + is a Data Science ``/apps`` record that a config-only delete would leave + running. The remote config is read again right before the delete, so a + failed read raises and nothing is deleted. + """ + remote = client.get_config_detail( + component_id=SANDBOXES_COMPONENT_ID, config_id=config_id, branch_id=branch_id + ) + configuration = remote.get("configuration") if isinstance(remote, dict) else None + parameters = configuration.get("parameters") if isinstance(configuration, dict) else None + app_id = parameters.get("id") if isinstance(parameters, dict) else None + return str(app_id) if app_id else None + + +def _app_backed_message(config_id: str, app_id: str) -> str: + return ( + f"Workspace {config_id} is not deleted: its config has parameters.id ({app_id}), so it " + "is backed by a Data Science app that a config delete would leave running. Delete it " + "in the Keboola UI." + ) + + +def _refuse_app_backed_workspace(client: Any, config_id: str, branch_id: int | None) -> None: + """Refuse to delete a workspace whose config points at a Data Science app.""" + app_id = _app_id(client, config_id, branch_id) + if app_id: + raise KeboolaApiError( + message=_app_backed_message(config_id, app_id), + status_code=0, + error_code=ErrorCode.VALIDATION_ERROR, + retryable=False, + ) + + +def _delete_session(client: Any, config_id: str, session_id: str, deleted: list[str]) -> None: + """Delete one session and record it in *deleted*; a 404 counts as deleted. + + The HTTP layer repeats a DELETE after a 5xx, so a lost 204 comes back as a + 404 on the repeat. Any other failure raises with the ids deleted so far. + """ + try: + client.delete_editor_session(session_id) + except KeboolaApiError as exc: + if exc.status_code != 404: + raise KeboolaApiError( + message=( + f"Workspace {config_id} was not deleted: SQL editor session {session_id} " + f"could not be deleted ({exc.message}). Sessions already deleted: " + f"{', '.join(deleted) or 'none'}." + ), + status_code=exc.status_code, + error_code=exc.error_code, + retryable=exc.retryable, + details={"deleted_session_ids": list(deleted), "failed_session_id": session_id}, + ) from exc + deleted.append(session_id) + + +def delete_remote_config(client: Any, change: dict[str, Any], branch_id: int | None) -> None: + """Delete a config on the remote; for a workspace, its sessions first. + + A workspace's sessions are deleted before its configuration, like + ``kbc remote workspace delete``. A workspace backed by a Data Science app + is refused. Any failure raises before the config delete, so the push + records the error (with the ids of the sessions already deleted) and keeps + the manifest entry. The deleted session ids are recorded on *change* + (``deleted_session_ids``), one by one; a failed config delete after the + sessions names them in its error. + """ + component_id = change["component_id"] + config_id = change["config_id"] + if component_id != SANDBOXES_COMPONENT_ID: + client.delete_config(component_id=component_id, config_id=config_id, branch_id=branch_id) + return + _refuse_app_backed_workspace(client, config_id, branch_id) + session_ids = list_workspace_sessions(client, {config_id}, branch_id)[config_id] + deleted: list[str] = [] + change["deleted_session_ids"] = deleted + for session_id in session_ids: + _delete_session(client, config_id, session_id, deleted) + try: + client.delete_config(component_id=component_id, config_id=config_id, branch_id=branch_id) + except KeboolaApiError as exc: + # The change is not in pushed_details on a failure, so the error is the + # only place that says which sessions are already gone. + raise KeboolaApiError( + message=( + f"Workspace {config_id}: the configuration could not be deleted ({exc.message}). " + f"Its SQL editor sessions were already deleted: {', '.join(deleted) or 'none'}." + ), + status_code=exc.status_code, + error_code=exc.error_code, + retryable=exc.retryable, + details={"deleted_session_ids": list(deleted)}, + ) from exc + + +def preview_deletes( + service: SyncService, + alias: str, + project_root: Path, + branch_override: int | None, + dry_result: dict[str, Any], +) -> None: + """``push --dry-run``: list the SQL editor sessions a delete would remove. + + Adds one ``warnings[]`` entry per deleted workspace with the session count + and ids (never session details). A workspace backed by a Data Science app + gets a ``workspace_delete_refused`` warning instead, as push refuses it. A + read or listing failure becomes a warning that says push would not delete + the workspace. ``dry_result["changes"]`` holds + what push would apply (``plan_push``), so without ``--force`` it has no + delete and nothing is previewed. No API call without a workspace delete. + """ + deletes = [ + c + for c in dry_result["changes"] + if c["change_type"] == "deleted" + and c["component_id"] == SANDBOXES_COMPONENT_ID + and not c.get("is_row") + ] + if not deletes: + return + project = service.resolve_projects([alias])[alias] + manifest = load_manifest(project_root) + branch_id = service._resolve_branch_id( + alias, manifest, project_root, branch_override=branch_override + ) + warnings = dry_result.setdefault("warnings", []) + client = service._client_factory(project.stack_url, project.token) + try: + with client: + app_ids = {c["config_id"]: _app_id(client, c["config_id"], branch_id) for c in deletes} + listed = {config_id for config_id, app_id in app_ids.items() if not app_id} + sessions = list_workspace_sessions(client, listed, branch_id) if listed else {} + except KeboolaApiError as exc: + for change in deletes: + warnings.append( + { + "change_type": "workspace_sessions_unknown", + "component_id": SANDBOXES_COMPONENT_ID, + "config_id": change["config_id"], + "message": ( + f"Cannot read workspace {change['config_id']} or list its SQL editor " + f"sessions: {exc.message}. Push would not delete this workspace." + ), + } + ) + return + for change in deletes: + app_id = app_ids[change["config_id"]] + if app_id: + warnings.append( + { + "change_type": "workspace_delete_refused", + "component_id": SANDBOXES_COMPONENT_ID, + "config_id": change["config_id"], + "message": f"Push will refuse this delete. {_app_backed_message(change['config_id'], app_id)}", + } + ) + continue + session_ids = sessions[change["config_id"]] + warnings.append( + { + "change_type": "workspace_sessions", + "component_id": SANDBOXES_COMPONENT_ID, + "config_id": change["config_id"], + "session_count": len(session_ids), + "session_ids": session_ids, + "message": ( + f"Deleting workspace {change['config_id']} also deletes " + f"{len(session_ids)} SQL editor session(s) and their workspaces: " + f"{', '.join(session_ids) or '-'}." + ), + } + ) + + +@dataclass(frozen=True) +class ClonedWorkspace: + """A workspace config a ``sync clone`` created, with its input table ids.""" + + config_id: str + path: str + name: str + sources: list[str] + + +def cloned_workspace( + config_id: str, path: str, local_data: dict[str, Any] +) -> ClonedWorkspace | None: + """The input tables of a cloned workspace's ``_config.yml``, or ``None`` if it has none.""" + storage_input = local_data.get("input") + tables = storage_input.get("tables") if isinstance(storage_input, dict) else None + if not isinstance(tables, list): + return None + sources = [str(t["source"]) for t in tables if isinstance(t, dict) and t.get("source")] + if not sources: + return None + name = str(local_data.get("name", "")) + return ClonedWorkspace(config_id=config_id, path=path, name=name, sources=sources) + + +def missing_input_table_warnings( + service: SyncService, target_alias: str, workspaces: list[ClonedWorkspace] +) -> list[dict[str, Any]]: + """``sync clone``: one warning per workspace whose input tables the target lacks. + + Clone creates buckets, never tables, so a cloned workspace can point at + tables that do not exist in the target yet. Called by + ``_sync_clone_warnings.collect_clone_warnings`` with the workspaces the + clone created; lists the target's tables once, only when there is one. + """ + if not workspaces: + return [] + project = service.resolve_projects([target_alias])[target_alias] + client = service._client_factory(project.stack_url, project.token) + try: + with client: + existing = {str(t.get("id", "")) for t in client.list_tables()} + except KeboolaApiError as exc: + return [ + { + "change_type": "workspace_input_check_failed", + "component_id": SANDBOXES_COMPONENT_ID, + "message": f"Cannot list the target's tables to check workspace inputs: {exc.message}", + } + ] + warnings: list[dict[str, Any]] = [] + for workspace in workspaces: + missing = [source for source in workspace.sources if source not in existing] + if missing: + warnings.append( + { + "change_type": "workspace_input_tables_missing", + "component_id": SANDBOXES_COMPONENT_ID, + "config_id": workspace.config_id, + "path": workspace.path, + "missing_tables": missing, + "message": ( + f"Workspace {workspace.name or workspace.path}: input table(s) " + f"missing in the target: {', '.join(missing)}. Clone creates buckets, " + "not tables; load the tables before you open the workspace." + ), + } + ) + return warnings diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index 585d78b25..dc15de5f8 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -17,7 +17,6 @@ from ..config_store import ConfigStore from ..constants import ( - ALWAYS_IGNORED_COMPONENTS, BRANCH_MAPPING_FILENAME, CONFIG_FILENAME, CONFIG_HASH_VERSION, @@ -68,6 +67,7 @@ save_manifest, ) from ..sync.naming import config_path, config_row_path, sanitize_name +from . import _sync_workspace as sql_workspaces from ._encryption import ( encrypt_secrets_in_config, find_plaintext_secret_keys, @@ -291,6 +291,7 @@ def init_sync( project_root: Path, git_branching: bool = False, adopt_existing: bool = False, + sync_workspaces: bool = False, ) -> dict[str, Any]: """Initialize a sync working directory for a project. @@ -304,6 +305,9 @@ def init_sync( adopt_existing: If True and a manifest already exists, validate it against the alias's project_id and normalise it (idempotent upgrade of a ``kbc``-written manifest) instead of refusing. + sync_workspaces: Set the manifest key ``syncWorkspaces`` (CLI-25), + so pull/diff/push/clone also handle shared SQL workspaces. On + ``adopt_existing`` it only turns the key on, never off. Returns: Dict with initialization stats and created file paths. @@ -322,7 +326,7 @@ def init_sync( manifest_path = keboola_dir / "manifest.json" if manifest_path.exists(): if adopt_existing: - return self._adopt_existing_manifest(alias, project_root, project) + return self._adopt_existing_manifest(alias, project_root, project, sync_workspaces) raise FileExistsError( f"Manifest already exists at {manifest_path}. " "Use 'sync pull' to update, 'sync init --adopt-existing' to adopt a " @@ -360,6 +364,7 @@ def init_sync( allowTargetEnv=True, gitBranching=git_branching_config, naming=ManifestNaming(), + syncWorkspaces=sync_workspaces, branches=[ ManifestBranch( id=default_branch_id, @@ -401,6 +406,7 @@ def init_sync( "api_host": api_host, "git_branching": git_branching, "default_branch": default_branch_name, + "sync_workspaces": sync_workspaces, "files_created": created_files, } @@ -409,6 +415,7 @@ def _adopt_existing_manifest( alias: str, project_root: Path, project: Any, + sync_workspaces: bool = False, ) -> dict[str, Any]: """Validate and normalise an existing manifest written by kbc or kbagent. @@ -434,6 +441,7 @@ def _adopt_existing_manifest( ) api_host = project.stack_url.replace("https://", "").rstrip("/") + existing.sync_workspaces = existing.sync_workspaces or sync_workspaces save_manifest(project_root, existing) return { @@ -443,6 +451,7 @@ def _adopt_existing_manifest( "api_host": api_host, "git_branching": existing.git_branching.enabled, "default_branch": existing.git_branching.default_branch, + "sync_workspaces": existing.sync_workspaces, "files_created": [], } @@ -475,21 +484,6 @@ def _local_files_match_pull_state( return False return True - @staticmethod - def _effective_ignored_components(manifest: Manifest) -> frozenset[str]: - """Components excluded from this working tree's sync operations. - - The hardcoded :data:`ALWAYS_IGNORED_COMPONENTS` plus the manifest's - ``ignoredComponents``, which was declared in the schema from day one - but read by nothing until issue #689. Computed ONCE per pull/diff and - threaded through every filtering site so the remote side, the local - side and the force-pull conflict guard can never disagree about what is - ignored -- a disagreement is what turns a tracked-but-unfetchable - config into a phantom "added" that ``sync push`` duplicates on the - remote, once per push. - """ - return ALWAYS_IGNORED_COMPONENTS | frozenset(manifest.ignored_components) - def pull( self, alias: str, @@ -548,6 +542,7 @@ def pull( samples_data: dict[str, str] = {} # table_id -> CSV string with client: components = client.list_components_with_configs(branch_id=branch_id) + components = sql_workspaces.scope_listing(components, manifest) self._ensure_branch_registered(manifest, branch_id, client) folder_map = self._fetch_config_folders(client, branch_id) @@ -647,8 +642,8 @@ def pull( } # Resolved once and shared by the conflict guard, the fetch loop and - # the stale-entry sweep below -- see ``_effective_ignored_components``. - ignored_components = self._effective_ignored_components(manifest) + # the stale-entry sweep below -- see ``effective_ignored_components``. + ignored_components = sql_workspaces.effective_ignored_components(manifest) # Entries the remote no longer lists (#792 A/C); their on-disk dirs are # reserved so a same-named new config cannot land in one. stale = find_stale_entries( @@ -1252,11 +1247,12 @@ def diff( client = self._client_factory(project.stack_url, project.token) with client: components = client.list_components_with_configs(branch_id=branch_id) + components = sql_workspaces.scope_listing(components, manifest) self._ensure_branch_registered(manifest, branch_id, client) # One ignored set for BOTH sides of this diff -- the remote lookups # below and the local scoping further down. - ignored_components = self._effective_ignored_components(manifest) + ignored_components = sql_workspaces.effective_ignored_components(manifest) # Build remote lookups: # remote_configs: "{component_id}/{config_id}" -> parent config data @@ -1650,6 +1646,7 @@ def push( dry_result["never_fetched"] = never_fetched if orphaned: dry_result["orphaned"] = orphaned + sql_workspaces.preview_deletes(self, alias, project_root, branch_override, dry_result) return dry_result projects = self.resolve_projects([alias]) @@ -1815,11 +1812,8 @@ def push( pushed_details.append(change) elif change_type == "deleted": - client.delete_config( - component_id=component_id, - config_id=config_id, - branch_id=branch_id, - ) + # A workspace's SQL editor sessions go first (CLI-25). + sql_workspaces.delete_remote_config(client, change, branch_id) # Remove from manifest manifest.configurations = [ c diff --git a/src/keboola_agent_cli/sync/manifest.py b/src/keboola_agent_cli/sync/manifest.py index 1c70d2f91..0f0eb6cd1 100644 --- a/src/keboola_agent_cli/sync/manifest.py +++ b/src/keboola_agent_cli/sync/manifest.py @@ -154,6 +154,10 @@ class Manifest(BaseModel): naming: ManifestNaming allowed_branches: list[str] = Field(default_factory=list, alias="allowedBranches") ignored_components: list[str] = Field(default_factory=list, alias="ignoredComponents") + # Opt-in (CLI-25): sync shared SQL workspaces (``keboola.sandboxes``) in + # this tree. kbagent-only: ``kbc`` ignores unknown manifest keys on load. + # ``save_manifest`` writes the key only while it is true. + sync_workspaces: bool = Field(default=False, alias="syncWorkspaces") branches: list[ManifestBranch] = Field(default_factory=list) configurations: list[ManifestConfiguration] = Field(default_factory=list) @@ -184,13 +188,16 @@ def save_manifest(project_root: Path, manifest: Manifest) -> None: """Save *manifest* to .keboola/manifest.json. Uses ``by_alias=True`` so all keys are written in camelCase, - matching the format expected by the Go CLI. + matching the format expected by the Go CLI. ``syncWorkspaces`` is left + out while false, so a tree that never opted in keeps its manifest as it was. """ keboola_dir = project_root / KEBOOLA_DIR_NAME keboola_dir.mkdir(parents=True, exist_ok=True) manifest_path = keboola_dir / MANIFEST_FILENAME payload = manifest.model_dump(mode="json", by_alias=True) + if not manifest.sync_workspaces: + payload.pop("syncWorkspaces", None) manifest_path.write_text( json.dumps(payload, indent=4, ensure_ascii=False) + "\n", encoding="utf-8", diff --git a/tests/test_sync_clone_links.py b/tests/test_sync_clone_links.py index 84593dbc3..fba0caeb8 100644 --- a/tests/test_sync_clone_links.py +++ b/tests/test_sync_clone_links.py @@ -702,6 +702,53 @@ def test_plain_push_of_a_fresh_tree_repoints_links(world: World) -> None: assert result["link_remaps"]["config_row_ids"] == 1 +def _with_shared_sql_workspace(components: list[dict[str, Any]]) -> None: + """A shared SQL workspace created from ``SQL step``: its shared-code and variables links.""" + _component_entry(components, SANDBOX)["configurations"].append( + _config( + "ws-ref", + "SQL workspace", + { + "parameters": _blocks(("Shared", [f"{{{{ {SHARED_ROW_ID} }}}}"])), + "runtime": {"shared": True}, + "shared_code_id": "sc-sql", + "shared_code_row_ids": [SHARED_ROW_ID], + "variables_id": "var-ref", + "variables_values_id": "vrow-ref", + }, + ) + ) + + +def test_clone_repoints_links_of_a_shared_sql_workspace( + tmp_path: Path, tmp_config_dir: Path +) -> None: + """A workspace created from a transformation carries its links in the config + body, like the transformation (CLI-25): push re-points them to the target.""" + world = World(tmp_path, tmp_config_dir, reference=_with_shared_sql_workspace) + manifest = load_manifest(world.ref_dir) + manifest.sync_workspaces = True + save_manifest(world.ref_dir, manifest) + world.service.pull(alias="ref", project_root=world.ref_dir, no_storage=True, no_jobs=True) + + result = world.clone() + + assert result["errors"] == [] + shared = world.target(SHARED, "Shared sql") + new_row_id = shared["rows"][0]["id"] + variables = world.target(VARS) + configuration = world.target(SANDBOX, "SQL workspace")["configuration"] + assert configuration["shared_code_id"] == shared["id"] + assert configuration["shared_code_row_ids"] == [new_row_id] + assert configuration["parameters"]["blocks"][0]["codes"][0]["script"] == [ + f"{{{{ {new_row_id} }}}}" + ] + assert configuration["variables_id"] == variables["id"] + assert configuration["variables_values_id"] == variables["rows"][0]["id"] + extra = world.local_config(SANDBOX, "SQL workspace")["_configuration_extra"] + assert extra["shared_code_id"] == shared["id"] + + # --------------------------------------------------------------------------- # Failure paths: every broken link is reported, and a failed PUT is retried # --------------------------------------------------------------------------- diff --git a/tests/test_sync_workspaces.py b/tests/test_sync_workspaces.py new file mode 100644 index 000000000..1c833dc76 --- /dev/null +++ b/tests/test_sync_workspaces.py @@ -0,0 +1,1374 @@ +"""Shared SQL workspaces in sync (CLI-25): pull, diff, push, clone. + +``keboola.sandboxes`` is always ignored unless the manifest sets +``syncWorkspaces``. With the key, only shared SQL workspaces (no +``parameters.id``, ``runtime.shared: true``) are synced, config only. A delete +removes the workspace's SQL editor sessions before the configuration. + +Every API call goes to a ``MagicMock(spec=KeboolaClient)`` client (or an +``httpx_mock`` transport for the Editor client); no test reaches a real project. +""" + +from __future__ import annotations + +import copy +import json +import shutil +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, call, patch + +import pytest +import yaml +from typer.testing import CliRunner + +from helpers import setup_single_project +from keboola_agent_cli.cli import app +from keboola_agent_cli.client import KeboolaClient +from keboola_agent_cli.config_store import CURRENT_CONFIG_VERSION +from keboola_agent_cli.constants import CONFIG_FILENAME, EXIT_PERMISSION_DENIED, KEBOOLA_DIR_NAME +from keboola_agent_cli.errors import ErrorCode, KeboolaApiError +from keboola_agent_cli.models import TokenVerifyResponse +from keboola_agent_cli.permissions import ( + FLAG_ESCALATIONS, + OPERATION_REGISTRY, + PermissionEngine, + PermissionPolicy, +) +from keboola_agent_cli.services._sync_push_ops import SKIPPED_DELETIONS_REASON +from keboola_agent_cli.services._sync_stale import WORKSPACE_SYNC_OFF_REASON +from keboola_agent_cli.services._sync_workspace import ( + SANDBOXES_COMPONENT_ID, + is_shared_sql_workspace, + list_workspace_sessions, +) +from keboola_agent_cli.services.project_service import ProjectService +from keboola_agent_cli.services.sync_service import SyncService +from keboola_agent_cli.sync.manifest import load_manifest, save_manifest + +BRANCH_ID = 12345 +DEV_BRANCH_ID = 99999 +TOKEN = "901-xxx" # setup_single_project's token +PRIVATE_KEY_MARKER = "-----BEGIN PRIVATE KEY-----fake-key-material" + +VERIFY_TOKEN = TokenVerifyResponse( + token_id="tok-001", + token_description="kbagent-cli", + project_id=258, + project_name="Production", + owner_name="My Org", +) +BRANCHES = [ + {"id": BRANCH_ID, "name": "Main", "isDefault": True}, + {"id": DEV_BRANCH_ID, "name": "feature-x", "isDefault": False}, +] + +# A SQL workspace config as the UI creates it (keboola/ui sandboxes helpers.ts): +# runtime.shared, parameters.backendSize, read_only_storage_access only when +# off, SQL blocks, and the links a from-transformation workspace carries. +SHARED_SQL: dict[str, Any] = { + "id": "ws-shared", + "name": "Shared SQL", + "description": "", + "configuration": { + "parameters": { + "blocks": [ + { + "name": "Block 1", + "codes": [{"name": "Code 1", "script": ["SELECT 1;\n\nSELECT 2;"]}], + } + ], + "backendSize": "small", + }, + "storage": { + "input": { + "tables": [{"source": "in.c-main.orders", "destination": "orders"}], + "read_only_storage_access": False, + }, + "output": {"tables": []}, + }, + "runtime": {"shared": True}, + "shared_code_id": "sc-1", + "shared_code_row_ids": ["sc-row-1"], + "variables_id": "var-1", + "variables_values_id": "var-values-1", + }, + "rows": [], +} +PRIVATE_SQL: dict[str, Any] = { + "id": "ws-private", + "name": "Private SQL", + "description": "", + "configuration": {"parameters": {"blocks": []}, "runtime": {"shared": False}}, + "rows": [], +} +PYTHON_WS: dict[str, Any] = { + "id": "ws-python", + "name": "Python", + "description": "", + "configuration": {"parameters": {"id": "5551234"}, "runtime": {"shared": True}}, + "rows": [], +} +EXTRACTOR: dict[str, Any] = { + "id": "keboola.ex-http", + "type": "extractor", + "configurations": [ + { + "id": "cfg-001", + "name": "My HTTP Extractor", + "description": "", + "configuration": {"parameters": {"baseUrl": "https://api.example.com"}}, + "rows": [], + } + ], +} +MCP_TOOL: dict[str, Any] = { + "id": "keboola.mcp-server-tool", + "type": "application", + "configurations": [ + {"id": "mcp-001", "name": "MCP", "description": "", "configuration": {}, "rows": []} + ], +} + +SHARED_SQL_PATH = "other/keboola.sandboxes/shared-sql" + + +def _sandboxes(*configs: dict[str, Any]) -> dict[str, Any]: + return {"id": SANDBOXES_COMPONENT_ID, "type": "other", "configurations": list(configs)} + + +def _remote(*configs: dict[str, Any]) -> list[dict[str, Any]]: + return [copy.deepcopy(EXTRACTOR), _sandboxes(*copy.deepcopy(list(configs)))] + + +def _client(components: list[dict[str, Any]] | None = None) -> MagicMock: + """A fake ``KeboolaClient``: ``spec`` makes a call to a method it lacks fail.""" + client = MagicMock(spec=KeboolaClient) + client.__enter__.return_value = client + client.__exit__.return_value = False + client.verify_token.return_value = VERIFY_TOKEN + client.list_dev_branches.return_value = BRANCHES + client.list_components_with_configs.return_value = components or [] + client.list_config_folder_metadata.return_value = {} + client.list_editor_sessions.return_value = [] + # The workspace read right before a delete (the parameters.id check). + client.get_config_detail.return_value = copy.deepcopy(SHARED_SQL) + return client + + +def _svc(store: Any, client: MagicMock) -> SyncService: + return SyncService(config_store=store, client_factory=lambda url, token: client) + + +def _init(tmp_config_dir: Path, root: Path, *, sync_workspaces: bool) -> Any: + store = setup_single_project(tmp_config_dir) + _svc(store, _client()).init_sync( + alias="prod", project_root=root, sync_workspaces=sync_workspaces + ) + return store + + +def _pull(store: Any, root: Path, components: list[dict[str, Any]]) -> dict[str, Any]: + return _svc(store, _client(components)).pull( + alias="prod", project_root=root, no_storage=True, no_jobs=True + ) + + +def _tracked(root: Path) -> set[str]: + return { + cfg.id + for cfg in load_manifest(root).configurations + if cfg.component_id == SANDBOXES_COMPONENT_ID + } + + +def _config_file(root: Path, rel_path: str = SHARED_SQL_PATH) -> Path: + return root / "main" / rel_path / CONFIG_FILENAME + + +def _edit(root: Path, edit: Any, rel_path: str = SHARED_SQL_PATH) -> None: + path = _config_file(root, rel_path) + data = yaml.safe_load(path.read_text(encoding="utf-8")) + edit(data) + path.write_text(yaml.dump(data, sort_keys=False), encoding="utf-8") + + +def _set_manifest_key(root: Path, key: str, value: Any) -> None: + path = root / KEBOOLA_DIR_NAME / "manifest.json" + data = json.loads(path.read_text(encoding="utf-8")) + if value is None: + data.pop(key, None) + else: + data[key] = value + path.write_text(json.dumps(data, indent=4), encoding="utf-8") + + +def _session( + session_id: str, + config_id: str, + *, + branch_id: int = BRANCH_ID, + component_id: str = SANDBOXES_COMPONENT_ID, +) -> dict[str, Any]: + """A session shaped like the editor-service swagger ``SqlEditorSession``. + + ``snowflakePrivateKey`` is present although the service sends it only with + ``includeCredentials``: the tests prove it never reaches kbagent's output. + """ + return { + "id": session_id, + "status": "ready", + "userId": "42", + "branchId": str(branch_id), + "componentId": component_id, + "configurationId": config_id, + "workspaceId": "9001", + "workspaceSchema": "WORKSPACE_9001", + "backendType": "snowflake", + "shared": True, + "snowflakePrivateKey": PRIVATE_KEY_MARKER, + } + + +# --------------------------------------------------------------------------- +# Manifest key +# --------------------------------------------------------------------------- + + +class TestManifestKey: + def test_key_written_only_when_on(self, tmp_config_dir: Path, tmp_path: Path) -> None: + off, on = tmp_path / "off", tmp_path / "on" + off.mkdir() + on.mkdir() + _init(tmp_config_dir, off, sync_workspaces=False) + store = setup_single_project(tmp_path / "cfg2") + result = _svc(store, _client()).init_sync( + alias="prod", project_root=on, sync_workspaces=True + ) + + assert result["sync_workspaces"] is True + assert "syncWorkspaces" not in json.loads((off / ".keboola" / "manifest.json").read_text()) + assert json.loads((on / ".keboola" / "manifest.json").read_text())["syncWorkspaces"] is True + + def test_save_keeps_unknown_keys(self, tmp_config_dir: Path, tmp_path: Path) -> None: + """Keys kbagent does not model (a kbc ``templates`` block, a future key) + survive a load/save round trip.""" + root = tmp_path / "project" + root.mkdir() + _init(tmp_config_dir, root, sync_workspaces=True) + _set_manifest_key(root, "templates", {"repositories": [{"name": "keboola"}]}) + _set_manifest_key(root, "someFutureKey", 7) + + save_manifest(root, load_manifest(root)) + + data = json.loads((root / ".keboola" / "manifest.json").read_text()) + assert data["templates"] == {"repositories": [{"name": "keboola"}]} + assert data["someFutureKey"] == 7 + assert data["syncWorkspaces"] is True + + def test_adopt_existing_turns_key_on(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=False) + + result = _svc(store, _client()).init_sync( + alias="prod", project_root=root, adopt_existing=True, sync_workspaces=True + ) + + assert result["status"] == "adopted" + assert result["sync_workspaces"] is True + assert load_manifest(root).sync_workspaces is True + + +# --------------------------------------------------------------------------- +# Which workspaces are in scope +# --------------------------------------------------------------------------- + + +class TestScopeRule: + def test_shared_sql_workspace(self) -> None: + assert is_shared_sql_workspace(SHARED_SQL) + + def test_empty_parameters_id_counts_as_sql(self) -> None: + cfg = {"configuration": {"parameters": {"id": ""}, "runtime": {"shared": True}}} + assert is_shared_sql_workspace(cfg) + + def test_not_shared_is_out(self) -> None: + assert not is_shared_sql_workspace(PRIVATE_SQL) + assert not is_shared_sql_workspace({"configuration": {"parameters": {}}}) + + def test_python_and_legacy_sql_sandbox_are_out(self) -> None: + """``parameters.id`` = a Data Science /apps record: Python/R, or a legacy + SQL sandbox from before the SQL editor. Both stay skipped.""" + assert not is_shared_sql_workspace(PYTHON_WS) + + +# --------------------------------------------------------------------------- +# Pull / diff +# --------------------------------------------------------------------------- + + +class TestPullDiff: + def test_opt_in_off_skips_all_workspaces(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=False) + + _pull(store, root, _remote(SHARED_SQL, PRIVATE_SQL, PYTHON_WS)) + + assert _tracked(root) == set() + assert not list(root.rglob("*keboola.sandboxes*")) + diff = _svc(store, _client(_remote(SHARED_SQL))).diff(alias="prod", project_root=root) + assert diff["summary"]["remote_only"] == 0 + + def test_opt_in_on_pulls_shared_sql_only(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + + _pull(store, root, _remote(SHARED_SQL, PRIVATE_SQL, PYTHON_WS)) + + assert _tracked(root) == {"ws-shared"} + local = yaml.safe_load(_config_file(root).read_text(encoding="utf-8")) + assert local["parameters"]["backendSize"] == "small" + assert local["parameters"]["blocks"][0]["codes"][0]["script"] == ["SELECT 1;\n\nSELECT 2;"] + assert local["input"]["read_only_storage_access"] is False + assert local["input"]["tables"][0]["source"] == "in.c-main.orders" + assert local["_configuration_extra"] == { + "runtime": {"shared": True}, + "shared_code_id": "sc-1", + "shared_code_row_ids": ["sc-row-1"], + "variables_id": "var-1", + "variables_values_id": "var-values-1", + } + + def test_no_phantom_diff_after_pull(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + remote = _remote(SHARED_SQL, PRIVATE_SQL, PYTHON_WS) + _pull(store, root, remote) + + diff = _svc(store, _client(remote)).diff(alias="prod", project_root=root) + + assert diff["changes"] == [] + assert diff["remote_only"] == [] + again = _pull(store, root, remote) + assert again["configs_pulled"] == 0 + + def test_ignored_components_entry_wins(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _set_manifest_key(root, "ignoredComponents", [SANDBOXES_COMPONENT_ID]) + + _pull(store, root, _remote(SHARED_SQL)) + + assert _tracked(root) == set() + + def test_mcp_server_tool_stays_ignored(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + + _pull(store, root, [*_remote(SHARED_SQL), copy.deepcopy(MCP_TOOL)]) + + components = {cfg.component_id for cfg in load_manifest(root).configurations} + assert components == {"keboola.ex-http", SANDBOXES_COMPONENT_ID} + + def test_turning_key_off_drops_entries_as_ignored( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + assert _config_file(root).exists() + + _set_manifest_key(root, "syncWorkspaces", None) + result = _pull(store, root, _remote(SHARED_SQL)) + + actions = { + d["action"] for d in result["details"] if d["component_id"] == SANDBOXES_COMPONENT_ID + } + assert actions == {"ignored"} + assert _tracked(root) == set() + assert not _config_file(root).exists() + + def test_turning_key_off_keeps_an_edited_workspace( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Un-pushed SQL in a workspace survives turning the key off: plain pull and + ``--force`` keep the directory and the entry (``skipped``); ``--theirs`` + deletes it.""" + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + _edit(root, lambda d: d["parameters"]["blocks"][0]["codes"][0].update(script=["SELECT 3;"])) + _set_manifest_key(root, "syncWorkspaces", None) + + for force in (False, True): + result = _svc(store, _client(_remote(SHARED_SQL))).pull( + alias="prod", project_root=root, force=force, no_storage=True, no_jobs=True + ) + [kept] = [d for d in result["details"] if d["component_id"] == SANDBOXES_COMPONENT_ID] + assert kept["action"] == "skipped" + assert kept["reason"] == WORKSPACE_SYNC_OFF_REASON + assert _tracked(root) == {"ws-shared"} + assert "SELECT 3;" in _config_file(root).read_text(encoding="utf-8") + + # Diff never plans a push for the kept entry: the component is ignored. + diff = _svc(store, _client(_remote(SHARED_SQL))).diff(alias="prod", project_root=root) + assert diff["changes"] == [] + + result = _svc(store, _client(_remote(SHARED_SQL))).pull( + alias="prod", project_root=root, theirs=True, no_storage=True, no_jobs=True + ) + assert {d["action"] for d in result["details"]} == {"ignored"} + assert _tracked(root) == set() + assert not _config_file(root).exists() + + def test_tracked_workspace_made_private_stays_tracked( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Someone turns ``runtime.shared`` off in the UI. Diff reports the remote + change; the entry does not vanish from the listing, which diff would read + as a local ``added`` and push would create a second workspace for.""" + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + made_private = copy.deepcopy(SHARED_SQL) + made_private["configuration"]["runtime"]["shared"] = False + client = _client(_remote(made_private)) + + diff = _svc(store, client).diff(alias="prod", project_root=root) + push = _svc(store, client).push(alias="prod", project_root=root) + + assert [c["change_type"] for c in diff["changes"]] == ["remote_modified"] + assert push["status"] == "no_changes" + client.create_config.assert_not_called() + _pull(store, root, _remote(made_private)) + assert _tracked(root) == {"ws-shared"} + + +# --------------------------------------------------------------------------- +# Push: create / update +# --------------------------------------------------------------------------- + + +def _write_new_workspace(root: Path) -> None: + config_dir = root / "main" / "other" / SANDBOXES_COMPONENT_ID / "new-ws" + config_dir.mkdir(parents=True) + (config_dir / CONFIG_FILENAME).write_text( + yaml.dump( + { + "version": 2, + "name": "New WS", + "parameters": {"blocks": [], "backendSize": "small"}, + "input": {"tables": [{"source": "in.c-main.orders", "destination": "orders"}]}, + "_configuration_extra": {"runtime": {"shared": True}}, + "_keboola": {"component_id": SANDBOXES_COMPONENT_ID, "config_id": ""}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + + +# Every client method a config-only push or clone may call: reads, and the +# Storage configuration writes. Anything else (a Queue job, a SQL editor +# session, a workspace load) fails ``_assert_config_only``. +_CONFIG_ONLY_CALLS = frozenset( + { + "__enter__", + "__exit__", + "verify_token", + "list_dev_branches", + "list_components_with_configs", + "list_config_folder_metadata", + "get_config_detail", + "create_config", + "update_config", + "set_config_metadata", + "list_tables", + } +) + + +def _assert_config_only(client: MagicMock) -> None: + """No Queue job, no SQL editor session, no table load: only config calls.""" + called = {name.split(".")[0].split("(")[0] for name, _args, _kwargs in client.mock_calls} + assert called <= _CONFIG_ONLY_CALLS, called - _CONFIG_ONLY_CALLS + client.create_job.assert_not_called() + client.load_workspace_tables.assert_not_called() + client.list_editor_sessions.assert_not_called() + + +def _assert_no_credentials(result: dict[str, Any]) -> None: + dumped = json.dumps(result, default=str) + assert TOKEN not in dumped + assert PRIVATE_KEY_MARKER not in dumped + assert "snowflakePrivateKey" not in dumped + + +class TestPushCreateUpdate: + def test_create_is_config_only(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote()) + _write_new_workspace(root) + client = _client(_remote()) + client.create_config.return_value = {"id": "ws-new", "name": "New WS"} + + result = _svc(store, client).push(alias="prod", project_root=root) + + assert result["created"] == 1, result + kwargs = client.create_config.call_args.kwargs + assert kwargs["component_id"] == SANDBOXES_COMPONENT_ID + assert kwargs["configuration"]["runtime"] == {"shared": True} + assert kwargs["configuration"]["parameters"]["backendSize"] == "small" + assert kwargs["branch_id"] == BRANCH_ID + _assert_config_only(client) + _assert_no_credentials(result) + assert _tracked(root) == {"ws-new"} + + def test_create_in_dev_branch(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote()) + _write_new_workspace(root) + client = _client(_remote()) + client.create_config.return_value = {"id": "ws-new"} + + _svc(store, client).push(alias="prod", project_root=root, branch_override=DEV_BRANCH_ID) + + assert client.create_config.call_args.kwargs["branch_id"] == DEV_BRANCH_ID + _assert_config_only(client) + + def test_update_warns_on_backend_size_change( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + _edit(root, lambda d: d["parameters"].update(backendSize="large")) + client = _client(_remote(SHARED_SQL)) + client.get_config_detail.return_value = copy.deepcopy(SHARED_SQL) + client.update_config.return_value = copy.deepcopy(SHARED_SQL) + + result = _svc(store, client).push(alias="prod", project_root=root) + + assert result["updated"] == 1, result + sent = client.update_config.call_args.kwargs["configuration"] + assert sent["parameters"]["backendSize"] == "large" + size_warnings = [ + w for w in result.get("warnings", []) if w["change_type"] == "workspace_backend_size" + ] + assert len(size_warnings) == 1 + assert size_warnings[0]["old_backend_size"] == "small" + assert size_warnings[0]["new_backend_size"] == "large" + assert "session created after this push" in size_warnings[0]["message"] + _assert_config_only(client) + _assert_no_credentials(result) + + def test_update_without_size_change_has_no_warning( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + _edit(root, lambda d: d["input"]["tables"].append({"source": "in.c-main.x"})) + client = _client(_remote(SHARED_SQL)) + client.get_config_detail.return_value = copy.deepcopy(SHARED_SQL) + client.update_config.return_value = copy.deepcopy(SHARED_SQL) + + result = _svc(store, client).push(alias="prod", project_root=root) + + assert result["updated"] == 1 + assert "warnings" not in result + + +# --------------------------------------------------------------------------- +# Push: delete (sessions first, then the config) +# --------------------------------------------------------------------------- + + +def _pulled_then_deleted_locally(tmp_config_dir: Path, root: Path) -> Any: + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + shutil.rmtree(_config_file(root).parent) + return store + + +def _sessions() -> list[dict[str, Any]]: + return [ + _session("s-mine", "ws-shared"), + _session("s-other-user", "ws-shared"), + _session("s-dev-branch", "ws-shared", branch_id=DEV_BRANCH_ID), + _session("s-other-config", "ws-other"), + _session("s-transformation", "ws-shared", component_id="keboola.snowflake-transformation"), + ] + + +class TestPushDelete: + def test_plain_push_holds_back_the_workspace_delete( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """Without --force push touches neither the sessions nor the config (#792 G).""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + + result = _svc(store, client).push(alias="prod", project_root=root) + + assert result["deleted"] == 0 + assert [c["config_id"] for c in result["skipped_deletions"]] == ["ws-shared"] + client.get_config_detail.assert_not_called() + client.list_editor_sessions.assert_not_called() + client.delete_editor_session.assert_not_called() + client.delete_config.assert_not_called() + assert _tracked(root) == {"ws-shared"} + + def test_held_back_workspace_delete_explains_force( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """The reason for a held-back workspace delete says what --force would also do.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + + result = _svc(store, _client(_remote(SHARED_SQL))).push(alias="prod", project_root=root) + + reason = result["skipped_deletions_reason"] + assert reason.startswith(SKIPPED_DELETIONS_REASON) + assert "SQL editor sessions of all users" in reason + assert "cannot be restored" in reason + assert "sync push --dry-run --force" in reason + + def test_held_back_plain_delete_has_no_workspace_note( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + extractor_path = next( + cfg.path for cfg in load_manifest(root).configurations if cfg.id == "cfg-001" + ) + shutil.rmtree(root / "main" / extractor_path) + + result = _svc(store, _client(_remote(SHARED_SQL))).push(alias="prod", project_root=root) + + assert [c["config_id"] for c in result["skipped_deletions"]] == ["cfg-001"] + assert result["skipped_deletions_reason"] == SKIPPED_DELETIONS_REASON + + def test_dry_run_flags_an_app_backed_workspace( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """The preview applies the parameters.id check of the real delete.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + app_backed = copy.deepcopy(SHARED_SQL) + app_backed["configuration"]["parameters"]["id"] = "5551234" + client.get_config_detail.return_value = app_backed + client.list_editor_sessions.return_value = _sessions() + + result = _svc(store, client).push(alias="prod", project_root=root, dry_run=True, force=True) + + [warning] = result["warnings"] + assert warning["change_type"] == "workspace_delete_refused" + assert "Push will refuse this delete" in warning["message"] + assert "5551234" in warning["message"] + client.list_editor_sessions.assert_not_called() + client.delete_config.assert_not_called() + + def test_plain_dry_run_previews_no_session_delete( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """The preview reads what plan_push would apply: no --force, no delete to preview.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + + result = _svc(store, client).push(alias="prod", project_root=root, dry_run=True) + + assert [c["config_id"] for c in result["skipped_deletions"]] == ["ws-shared"] + assert not [ + w for w in result.get("warnings", []) if w["change_type"].startswith("workspace") + ] + client.list_editor_sessions.assert_not_called() + + def test_force_delete_removes_sessions_then_config( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + order = MagicMock() + order.attach_mock(client.delete_editor_session, "delete_editor_session") + order.attach_mock(client.delete_config, "delete_config") + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + assert result["deleted"] == 1, result + client.list_editor_sessions.assert_called_once_with(branch_id=BRANCH_ID) + assert order.mock_calls == [ + call.delete_editor_session("s-mine"), + call.delete_editor_session("s-other-user"), + call.delete_config( + component_id=SANDBOXES_COMPONENT_ID, config_id="ws-shared", branch_id=BRANCH_ID + ), + ] + assert result["pushed_details"][0]["deleted_session_ids"] == ["s-mine", "s-other-user"] + assert _tracked(root) == set() + _assert_no_credentials(result) + + def test_session_list_failure_keeps_config(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.side_effect = KeboolaApiError( + message="Editor service unavailable", + status_code=503, + error_code=ErrorCode.API_ERROR, + ) + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + client.delete_config.assert_not_called() + client.delete_editor_session.assert_not_called() + assert result["deleted"] == 0 + assert result["errors"][0]["config_id"] == "ws-shared" + assert "Editor service unavailable" in result["errors"][0]["message"] + assert _tracked(root) == {"ws-shared"} + + def test_session_delete_failure_keeps_config( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + client.delete_editor_session.side_effect = KeboolaApiError( + message="Cannot delete session during initialization.", + status_code=400, + error_code=ErrorCode.API_ERROR, + ) + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + client.delete_config.assert_not_called() + assert result["errors"][0]["config_id"] == "ws-shared" + assert _tracked(root) == {"ws-shared"} + + def test_partial_session_delete_failure_reports_deleted_ids( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + client.delete_editor_session.side_effect = [ + None, + KeboolaApiError( + message="Server error", status_code=500, error_code=ErrorCode.API_ERROR + ), + ] + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + client.delete_config.assert_not_called() + [error] = result["errors"] + assert error["config_id"] == "ws-shared" + assert "s-other-user could not be deleted" in error["message"] + assert "Sessions already deleted: s-mine." in error["message"] + assert _tracked(root) == {"ws-shared"} + + def test_session_already_gone_counts_as_deleted( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """A retried DELETE after a lost 204 answers 404: the session is gone.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + client.delete_editor_session.side_effect = [ + KeboolaApiError(message="Not found", status_code=404, error_code=ErrorCode.NOT_FOUND), + None, + ] + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + assert result["errors"] == [] + assert result["deleted"] == 1 + assert result["pushed_details"][0]["deleted_session_ids"] == ["s-mine", "s-other-user"] + client.delete_config.assert_called_once() + + def test_failed_config_delete_names_the_deleted_sessions( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """The sessions are gone when the config delete fails; the error says so.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + client.delete_config.side_effect = KeboolaApiError( + message="Storage timed out", status_code=504, error_code=ErrorCode.TIMEOUT + ) + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + assert client.delete_editor_session.call_count == 2 + [error] = result["errors"] + assert error["config_id"] == "ws-shared" + assert "Storage timed out" in error["message"] + assert "already deleted: s-mine, s-other-user." in error["message"] + assert _tracked(root) == {"ws-shared"} + + def test_no_default_branch_deletes_nothing( + self, tmp_config_dir: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Production resolves to branch None (e.g. git-branching on the default git + branch). Without a default branch id the sessions cannot be filtered by + branch, so push must refuse instead of deleting the config and leaving + the sessions running.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + client.list_dev_branches.return_value = [{"id": DEV_BRANCH_ID, "isDefault": False}] + svc = _svc(store, client) + monkeypatch.setattr(svc, "_resolve_branch_id", lambda *args, **kwargs: None) + + result = svc.push(alias="prod", project_root=root, force=True) + + [error] = result["errors"] + assert error["config_id"] == "ws-shared" + assert "default branch" in error["message"] + client.list_editor_sessions.assert_not_called() + client.delete_editor_session.assert_not_called() + client.delete_config.assert_not_called() + assert _tracked(root) == {"ws-shared"} + + def test_app_backed_workspace_is_not_deleted( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + """A tracked workspace whose config now has parameters.id is backed by a Data + Science app; a config-only delete would leave the app running.""" + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + app_backed = copy.deepcopy(SHARED_SQL) + app_backed["configuration"]["parameters"]["id"] = "5551234" + client.get_config_detail.return_value = app_backed + + result = _svc(store, client).push(alias="prod", project_root=root, force=True) + + [error] = result["errors"] + assert error["error_code"] == ErrorCode.VALIDATION_ERROR + assert "parameters.id" in error["message"] + client.list_editor_sessions.assert_not_called() + client.delete_editor_session.assert_not_called() + client.delete_config.assert_not_called() + assert _tracked(root) == {"ws-shared"} + + def test_dry_run_lists_sessions_and_deletes_nothing( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.return_value = _sessions() + + result = _svc(store, client).push(alias="prod", project_root=root, dry_run=True, force=True) + + assert result["status"] == "dry_run" + [preview] = [w for w in result["warnings"] if w["change_type"] == "workspace_sessions"] + assert preview["config_id"] == "ws-shared" + assert preview["session_count"] == 2 + assert preview["session_ids"] == ["s-mine", "s-other-user"] + client.delete_editor_session.assert_not_called() + client.delete_config.assert_not_called() + _assert_no_credentials(result) + + def test_dry_run_reports_listing_failure(self, tmp_config_dir: Path, tmp_path: Path) -> None: + root = tmp_path / "project" + root.mkdir() + store = _pulled_then_deleted_locally(tmp_config_dir, root) + client = _client(_remote(SHARED_SQL)) + client.list_editor_sessions.side_effect = KeboolaApiError( + message="Editor service unavailable", status_code=503 + ) + + result = _svc(store, client).push(alias="prod", project_root=root, dry_run=True, force=True) + + [warning] = result["warnings"] + assert warning["change_type"] == "workspace_sessions_unknown" + assert "would not delete" in warning["message"] + + def test_dry_run_without_workspace_delete_makes_no_editor_call( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + root = tmp_path / "project" + root.mkdir() + store = _init(tmp_config_dir, root, sync_workspaces=True) + _pull(store, root, _remote(SHARED_SQL)) + _edit(root, lambda d: d["parameters"].update(backendSize="large")) + client = _client(_remote(SHARED_SQL)) + + result = _svc(store, client).push(alias="prod", project_root=root, dry_run=True) + + assert result["status"] == "dry_run" + assert "warnings" not in result + client.list_editor_sessions.assert_not_called() + + +class TestSessionListing: + def test_production_without_branch_id_resolves_default_branch(self) -> None: + client = _client() + client.list_editor_sessions.return_value = _sessions() + + sessions = list_workspace_sessions(client, {"ws-shared"}, None) + + client.list_editor_sessions.assert_called_once_with(branch_id=BRANCH_ID) + assert sessions == {"ws-shared": ["s-mine", "s-other-user"]} + + def test_dev_branch_sessions_only(self) -> None: + client = _client() + client.list_editor_sessions.return_value = _sessions() + + sessions = list_workspace_sessions(client, {"ws-shared"}, DEV_BRANCH_ID) + + assert sessions == {"ws-shared": ["s-dev-branch"]} + + +# --------------------------------------------------------------------------- +# Clone +# --------------------------------------------------------------------------- + + +class TestClone: + def test_clone_creates_workspace_and_warns_on_missing_tables( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + source = tmp_path / "golden" + source.mkdir() + store = _init(tmp_config_dir, source, sync_workspaces=True) + _pull(store, source, _remote(SHARED_SQL)) + client = _client([]) # fresh target + new_ids = {"keboola.ex-http": "ext-new", SANDBOXES_COMPONENT_ID: "ws-new"} + client.create_config.side_effect = lambda **kw: {"id": new_ids[kw["component_id"]]} + client.list_tables.return_value = [{"id": "in.c-prod.customers"}] + + result = _svc(store, client).clone_project( + source=source, + target_alias="prod", + target_dir=tmp_path / "clone", + overrides={"bucket_map": {"in.c-main": "in.c-prod"}, "create_buckets": False}, + ) + + assert result["status"] == "cloned", result + created = {c.kwargs["component_id"]: c.kwargs for c in client.create_config.call_args_list} + workspace = created[SANDBOXES_COMPONENT_ID]["configuration"] + # bucket_map rewrote the workspace input mapping like any other config. + assert workspace["storage"]["input"]["tables"][0]["source"] == "in.c-prod.orders" + [warning] = result["warnings"] + assert warning["change_type"] == "workspace_input_tables_missing" + assert warning["missing_tables"] == ["in.c-prod.orders"] + assert warning["config_id"] == "ws-new" + _assert_config_only(client) + + def test_clone_without_workspaces_makes_no_table_call( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + source = tmp_path / "golden" + source.mkdir() + store = _init(tmp_config_dir, source, sync_workspaces=False) + _pull(store, source, _remote(SHARED_SQL)) + client = _client([]) + client.create_config.return_value = {"id": "ext-new"} + + result = _svc(store, client).clone_project( + source=source, + target_alias="prod", + target_dir=tmp_path / "clone", + overrides={"create_buckets": False}, + ) + + assert result["warnings"] == [] + client.list_tables.assert_not_called() + + +# --------------------------------------------------------------------------- +# CLI + Editor client +# --------------------------------------------------------------------------- + + +class TestSyncInitCli: + def test_with_workspaces_is_forwarded(self, tmp_config_dir: Path, tmp_path: Path) -> None: + store = setup_single_project(tmp_config_dir) + svc = MagicMock() + svc.init_sync.return_value = { + "status": "initialized", + "project_id": 258, + "project_alias": "prod", + "api_host": "connection.keboola.com", + "git_branching": False, + "default_branch": "main", + "sync_workspaces": True, + "files_created": [], + } + with ( + patch("keboola_agent_cli.cli.ConfigStore", return_value=store), + patch( + "keboola_agent_cli.cli.ProjectService", + return_value=ProjectService(config_store=store), + ), + patch("keboola_agent_cli.cli.SyncService", return_value=svc), + ): + result = CliRunner().invoke( + app, + [ + "--json", + "sync", + "init", + "--project", + "prod", + "--directory", + str(tmp_path), + "--with-workspaces", + ], + ) + + assert result.exit_code == 0, result.output + assert svc.init_sync.call_args.kwargs["sync_workspaces"] is True + assert json.loads(result.output)["data"]["sync_workspaces"] is True + + +STACK_URL = "https://connection.eu-central-1.keboola.com" +EDITOR_URL = "https://editor.eu-central-1.keboola.com" +CLIENT_TOKEN = "901-55555-fakeTestTokenDoNotUseXXXXXXXX" + + +class TestEditorClient: + def test_list_sessions_of_all_users_in_branch(self, httpx_mock: Any) -> None: + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions?listAll=1&branchId={BRANCH_ID}", + method="GET", + json=[_session("s-1", "ws-shared")], + ) + + with KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client: + sessions = client.list_editor_sessions(branch_id=BRANCH_ID) + + assert [s["id"] for s in sessions] == ["s-1"] + request = httpx_mock.get_requests()[0] + assert request.headers["X-StorageApi-Token"] == CLIENT_TOKEN + assert "includeCredentials" not in str(request.url) + + def test_delete_session(self, httpx_mock: Any) -> None: + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions/s%2F1", method="DELETE", status_code=204 + ) + + with KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client: + client.delete_editor_session("s/1") + + assert str(httpx_mock.get_requests()[0].url).endswith("/sql/sessions/s%2F1") + + def test_non_list_body_raises(self, httpx_mock: Any) -> None: + """An unexpected 200 body must not read as "no sessions" (fail closed).""" + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions?listAll=1", + method="GET", + json={"sessions": [_session("s-1", "ws-shared")]}, + ) + + with ( + KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client, + pytest.raises(KeboolaApiError) as exc_info, + ): + client.list_editor_sessions() + + assert exc_info.value.error_code == ErrorCode.API_ERROR + + def test_invalid_json_raises_api_error(self, httpx_mock: Any) -> None: + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions?listAll=1", method="GET", text="proxy" + ) + + with ( + KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client, + pytest.raises(KeboolaApiError) as exc_info, + ): + client.list_editor_sessions() + + assert exc_info.value.error_code == ErrorCode.API_ERROR + + def test_list_with_a_non_object_item_raises(self, httpx_mock: Any) -> None: + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions?listAll=1", method="GET", json=["s-1"] + ) + + with ( + KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client, + pytest.raises(KeboolaApiError), + ): + client.list_editor_sessions() + + def test_list_forbidden_raises(self, httpx_mock: Any) -> None: + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions?listAll=1", + method="GET", + status_code=403, + json={"error": "You do not have permission to modify sessions.", "code": 403}, + ) + + with ( + KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client, + pytest.raises(KeboolaApiError) as exc_info, + ): + client.list_editor_sessions() + + assert exc_info.value.status_code == 403 + + def test_delete_404_raises_with_status(self, httpx_mock: Any) -> None: + """The service layer reads a 404 on delete as "already gone"; the client + reports the status so it can.""" + httpx_mock.add_response( + url=f"{EDITOR_URL}/sql/sessions/s-1", + method="DELETE", + status_code=404, + json={"error": "Session not found", "code": 404}, + ) + + with ( + KeboolaClient(stack_url=STACK_URL, token=CLIENT_TOKEN) as client, + pytest.raises(KeboolaApiError) as exc_info, + ): + client.delete_editor_session("s-1") + + assert exc_info.value.status_code == 404 + + +# --------------------------------------------------------------------------- +# Human output: warnings are plain text, never Rich markup +# --------------------------------------------------------------------------- + +BRACKETED = [ + {"change_type": "workspace_sessions", "message": "Workspace Sales [prod] mart: 1 session"}, + {"change_type": "workspace_sessions", "message": "Workspace Tmp [/wip] copy: 2 sessions"}, +] + + +def _invoke_with_sync_service(tmp_config_dir: Path, svc: MagicMock, args: list[str]) -> Any: + store = setup_single_project(tmp_config_dir) + with ( + patch("keboola_agent_cli.cli.ConfigStore", return_value=store), + patch( + "keboola_agent_cli.cli.ProjectService", + return_value=ProjectService(config_store=store), + ), + patch("keboola_agent_cli.cli.SyncService", return_value=svc), + ): + return CliRunner().invoke(app, args) + + +def _stderr_text(result: Any) -> str: + return " ".join(result.stderr.split()) + + +class TestHumanWarnings: + def test_push_dry_run_prints_bracketed_names( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + svc = MagicMock() + svc.push.return_value = { + "status": "dry_run", + "changes": [], + "summary": {"added": 0, "modified": 0, "deleted": 1}, + "warnings": BRACKETED, + } + + result = _invoke_with_sync_service( + tmp_config_dir, + svc, + ["sync", "push", "--project", "prod", "--directory", str(tmp_path), "--dry-run"], + ) + + assert result.exit_code == 0, result.output + stderr = _stderr_text(result) + assert "Workspace Sales [prod] mart: 1 session" in stderr + assert "Workspace Tmp [/wip] copy: 2 sessions" in stderr + + def test_push_all_projects_verbose_prints_warnings( + self, tmp_config_dir: Path, tmp_path: Path + ) -> None: + svc = MagicMock() + svc.push_all.return_value = { + "summary": {"total": 1, "success": 1, "failed": 0}, + "projects": { + "prod": { + "status": "pushed", + "created": 0, + "updated": 1, + "deleted": 0, + "errors": [], + "warnings": [ + { + "change_type": "workspace_backend_size", + "message": "Workspace [ws-1]: backendSize changed", + } + ], + } + }, + "skipped": [], + } + + result = _invoke_with_sync_service( + tmp_config_dir, + svc, + ["--verbose", "sync", "push", "--all-projects", "--directory", str(tmp_path)], + ) + + assert result.exit_code == 0, result.output + assert "Workspace [ws-1]: backendSize changed" in _stderr_text(result) + + def test_clone_prints_bracketed_names(self, tmp_config_dir: Path, tmp_path: Path) -> None: + source = tmp_path / "golden" + (source / ".keboola").mkdir(parents=True) + svc = MagicMock() + svc.clone_project.return_value = { + "status": "cloned", + "target_alias": "prod", + "target_dir": str(tmp_path / "clone"), + "created": 1, + "errors": [], + "warnings": BRACKETED, + } + + result = _invoke_with_sync_service( + tmp_config_dir, + svc, + [ + "sync", + "clone", + "--source", + str(source), + "--target", + "prod", + "--target-dir", + str(tmp_path / "clone"), + ], + ) + + assert result.exit_code == 0, result.output + stderr = _stderr_text(result) + assert "Workspace Sales [prod] mart: 1 session" in stderr + assert "Workspace Tmp [/wip] copy: 2 sessions" in stderr + + +# --------------------------------------------------------------------------- +# Permissions: `sync push --force` is destructive +# --------------------------------------------------------------------------- + + +class TestPushForcePermission: + def test_force_is_a_destructive_escalation(self) -> None: + assert FLAG_ESCALATIONS["sync.push --force"] == "destructive" + assert "sync.push --force" not in OPERATION_REGISTRY + engine = PermissionEngine(PermissionPolicy(mode="allow", deny=["cli:destructive"])) + assert engine.is_allowed("sync.push") is True + assert engine.is_allowed("sync.push --force") is False + + def _config_dir(self, tmp_path: Path, deny: list[str]) -> Path: + config_dir = tmp_path / "c" + config_dir.mkdir() + # Written directly: `permissions set` needs a human at a real terminal. + (config_dir / "config.json").write_text( + json.dumps( + { + "version": CURRENT_CONFIG_VERSION, + "projects": {}, + "permissions": {"mode": "allow", "allow": [], "deny": deny}, + } + ) + ) + return config_dir + + def test_policy_denying_destructive_blocks_force_only(self, tmp_path: Path) -> None: + config_dir = self._config_dir(tmp_path, ["cli:destructive"]) + + denied, svc = self._push_with(config_dir, tmp_path, "--force") + assert denied.exit_code == EXIT_PERMISSION_DENIED + svc.push.assert_not_called() + + allowed, svc = self._push_with(config_dir, tmp_path) + assert allowed.exit_code == 0, allowed.output + svc.push.assert_called_once() + + def test_deny_destructive_flag_blocks_force(self, tmp_path: Path) -> None: + config_dir = self._config_dir(tmp_path, []) + svc = MagicMock() + with patch("keboola_agent_cli.cli.SyncService", return_value=svc): + result = CliRunner().invoke( + app, + [ + "--config-dir", + str(config_dir), + "--deny-destructive", + "--json", + "sync", + "push", + "--project", + "prod", + "--directory", + str(tmp_path), + "--force", + ], + ) + assert result.exit_code == EXIT_PERMISSION_DENIED + svc.push.assert_not_called() + + def _push_with(self, config_dir: Path, tmp_path: Path, *flags: str) -> tuple[Any, MagicMock]: + svc = MagicMock() + svc.push.return_value = {"status": "no_changes", "created": 0, "updated": 0, "deleted": 0} + with patch("keboola_agent_cli.cli.SyncService", return_value=svc): + result = CliRunner().invoke( + app, + [ + "--config-dir", + str(config_dir), + "--json", + "sync", + "push", + "--project", + "prod", + "--directory", + str(tmp_path), + *flags, + ], + ) + return result, svc From 230a3b02f3d89e1e29ae8b6521509e284e470037 Mon Sep 17 00:00:00 2001 From: soustruh Date: Wed, 30 Sep 2026 09:36:41 +0200 Subject: [PATCH 2/2] docs(permissions): an allow-list must name sync.push --force, pin it with a test (CLI-25) --- .../skills/kbagent/references/gotchas.md | 4 ++ .../references/permissions-workflow.md | 2 + tests/test_sync_workspaces.py | 42 ++++++++++++++++++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 390c02a16..4aa23f545 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -5553,6 +5553,10 @@ workspaces". - **`sync push --force` is destructive-class** (operation `sync.push --force` in `permissions list`): a policy denying `cli:destructive`, or `--deny-destructive`, blocks it, while a plain `sync push` stays write-class. + An allow-list that names only `sync.push` now blocks `sync push --force` + too (also in trees without `syncWorkspaces`): add + `--allow "sync.push --force"` or a glob such as `sync.*`. The same applies to + a default-allow policy that denies `cli:write` and allows `sync.push`. - **Removing the key** makes the next `sync pull` drop the workspace entries with action `ignored`, except a workspace edited locally and not pushed: pull (also `--force`) keeps it and reports it as `skipped`; only `--theirs` diff --git a/plugins/kbagent/skills/kbagent/references/permissions-workflow.md b/plugins/kbagent/skills/kbagent/references/permissions-workflow.md index ead9ddcee..7567a2c6d 100644 --- a/plugins/kbagent/skills/kbagent/references/permissions-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/permissions-workflow.md @@ -88,6 +88,8 @@ kbagent permissions set --mode deny \ ``` Everything else is blocked. This is the most restrictive approach. +*(since vNEXT)* `sync push --force` is checked as its own operation, `sync.push --force`. An allow-list that names only `sync.push` allows a plain push and blocks a forced push (exit 6). To allow a forced push, add `--allow "sync.push --force"`, or use a glob such as `sync.*`. The same is true for a default-allow policy that denies `cli:write` and allows `sync.push`. + ## Checking permissions before acting ```bash diff --git a/tests/test_sync_workspaces.py b/tests/test_sync_workspaces.py index 1c833dc76..a6fa4f141 100644 --- a/tests/test_sync_workspaces.py +++ b/tests/test_sync_workspaces.py @@ -1303,7 +1303,34 @@ def test_force_is_a_destructive_escalation(self) -> None: assert engine.is_allowed("sync.push") is True assert engine.is_allowed("sync.push --force") is False - def _config_dir(self, tmp_path: Path, deny: list[str]) -> Path: + @pytest.mark.parametrize( + ("mode", "allow", "deny", "force_allowed"), + [ + # An allow-list that names only `sync.push` allows a plain push, not `--force`. + ("deny", ["sync.push"], [], False), + ("deny", ["sync.push", "sync.push --force"], [], True), + ("deny", ["sync.*"], [], True), + ("deny", ["sync.push", "cli:destructive"], [], True), + # `cli:write` covers the destructive class, and `sync.push` does not match `--force`. + ("allow", ["sync.push"], ["cli:write"], False), + ("allow", ["sync.push", "sync.push --force"], ["cli:write"], True), + ], + ) + def test_allow_list_must_name_the_force_escalation( + self, mode: str, allow: list[str], deny: list[str], force_allowed: bool + ) -> None: + engine = PermissionEngine(PermissionPolicy(mode=mode, allow=allow, deny=deny)) + assert engine.is_allowed("sync.push") is True + assert engine.is_allowed("sync.push --force") is force_allowed + + def _config_dir( + self, + tmp_path: Path, + deny: list[str], + *, + mode: str = "allow", + allow: list[str] | None = None, + ) -> Path: config_dir = tmp_path / "c" config_dir.mkdir() # Written directly: `permissions set` needs a human at a real terminal. @@ -1312,12 +1339,23 @@ def _config_dir(self, tmp_path: Path, deny: list[str]) -> Path: { "version": CURRENT_CONFIG_VERSION, "projects": {}, - "permissions": {"mode": "allow", "allow": [], "deny": deny}, + "permissions": {"mode": mode, "allow": allow or [], "deny": deny}, } ) ) return config_dir + def test_allow_list_with_only_sync_push_blocks_force(self, tmp_path: Path) -> None: + config_dir = self._config_dir(tmp_path, [], mode="deny", allow=["sync.push"]) + + denied, svc = self._push_with(config_dir, tmp_path, "--force") + assert denied.exit_code == EXIT_PERMISSION_DENIED + svc.push.assert_not_called() + + allowed, svc = self._push_with(config_dir, tmp_path) + assert allowed.exit_code == 0, allowed.output + svc.push.assert_called_once() + def test_policy_denying_destructive_blocks_force_only(self, tmp_path: Path) -> None: config_dir = self._config_dir(tmp_path, ["cli:destructive"])