fix(sync): keep a data app's runtime type through pull and clone (CLI-8)

[soustruh]

Why

A data app's runtime type (python-js, streamlit, ...) lives only on the Data Science /apps record, never in the Storage config. sync pull read only the Storage config, so it dropped the type. sync push and sync clone recreated the config through the Storage API alone. A cloned python-js app then deployed as the platform default, streamlit. The bug report was about sync clone. The same gap exists on plain sync push.

What changed

Two changes, both limited to keboola.data-apps:

• pull reads the type from the Data Science /apps list and writes it into the config's _keboola metadata block as data_app_type. That block holds kbagent's own bookkeeping in each _config.yml. It never reaches the Storage API, and kbagent excludes it from the config hash, so the new line produces no sync diff change on existing trees.
• push creates a keboola.data-apps config through the Data Science create_app call. That call creates the DS record with the type and its Storage config. push then fills the body with update_config and updates the parameters.id link to the new app. Without a recorded type, push uses the plain create_config path unchanged.

The DS create logic lives in data_app_service.create_synced_data_app. push_create only delegates for that one component. sync clone uses the same push().

The /apps list is not only data apps

list_apps() also returns sandbox and workspace records. Each carries a parent component's id (for example keboola.ex-db-mysql) and a backend type such as snowflake. So kbagent builds the type map from componentId == keboola.data-apps records only. It writes the type into keboola.data-apps configs only. A live pull of project 4214 confirmed this. It set the type on 21 data apps (14 python-js, 7 streamlit) and added it to no other config.

Testing

• New tests/test_sync_data_app_type.py:
  • pull writes the type into _keboola.
  • the map filter and the write guard defend against sandbox records and against a config id shared across components.
  • push sends the type and updates parameters.id.
  • a push with no recorded type uses create_config instead.
• Added create_synced_data_app unit tests, plus api_config_to_local tests for the type and for hash-invisibility.
• Full sync and data-app suites pass. ruff 0.16.5, ruff format, and ty are clean.
• Live verification on project 4214 was read-only (pull only, no writes to any project).

Scope note

A data-apps config's is_disabled flag does not travel through the DS create path, so a cloned app deploys enabled. This is out of scope for CLI-8. I can do a follow-up if you want.

Linear: CLI-8

[linear-code]

CLI-8

[soustruh]

Review of #752 — fix(sync): keep a data app's runtime type through pull and clone (CLI-8)

Generated by kbagent-pr-reviewer subagent. Verdict and findings below are advisory; the human author retains every veto. CI-coverable issues (lint, format, tests) are confirmed via make check, not duplicated here.

Summary

This PR fixes a real bug: sync pull dropped a keboola.data-apps config's runtime type (python-js/streamlit), and sync push/sync clone then recreated the config under the platform default. The fix stores the type in the local _config.yml's hash-ignored _keboola footer on pull, and routes a data-app CREATE with a recorded type through a new Data-Science-aware helper on push. The core logic is sound and its field-shape assumptions check out against a live project, but the PR cannot merge as submitted: it grows an already-over-budget service file past its grandfathered file-size ceiling, which fails CI's file-size gate directly, and it changes non-obvious sync behavior with no corresponding entry in plugins/kbagent/skills/kbagent/references/gotchas.md.

Verdict

• Verdict: REQUEST CHANGES
• Blocking findings: 2
• Non-blocking findings: 4
• Nits: 1

Blocking findings

[B-1] src/keboola_agent_cli/services/data_app_service.py:144-206 — new create_synced_data_app pushes the file past its grandfathered file-size ceiling

services/data_app_service.py is already over its 1500-line hard ceiling and grandfathered at 1641 code lines in scripts/file_size_baseline.json. The new create_synced_data_app function (lines 144-206) grows the file to 1679 code lines, which fails make loc-check locally (FAIL: services/data_app_service.py grew to 1679 code lines, past its grandfathered 1641) and tests/test_check_file_size.py::TestRepoState::test_repo_passes_its_own_gate. scripts/check_file_size.py runs as its own step in .github/workflows/ci.yml's check job, so this is a certain CI failure, not just a local nuance. Move create_synced_data_app into a sibling module (e.g. _sync_data_app_ops.py, next to the existing _sync_push_ops.py) or otherwise shrink the file below 1641 before merging.

[B-2] src/keboola_agent_cli/services/sync_service.py:588-601 + _sync_push_ops.py:434-446 — no gotchas.md entry for the new data-app-type pull/push behavior

The diff touches no file under plugins/kbagent/. This PR changes the observable behavior of sync pull (writes a new _keboola.data_app_type key) and sync push/sync clone (routes data-app creates through the Data Science API when a type is recorded) for keboola.data-apps configs — exactly the class of change CONTRIBUTING.md's Plugin synchronization map calls out for gotchas.md: "New non-obvious behavior -- always tag with a version," rated BLOCKING when missing because an untagged behavior change means an AI agent following the existing data-app gotchas (which already document several other data-app silent-failure modes, e.g. "Data app Storage access fails SILENTLY") never learns about this one. Add a (since vNEXT)-tagged entry; while there, a matching one-liner in keboola-expert.md §3 and the sync pull/sync push bullets in commands-reference.md would close the loop.

Non-blocking findings

[NB-1] src/keboola_agent_cli/services/data_app_service.py:198-206 — is_disabled is dropped on the new DS-routed create path

create_synced_data_app's update_config call omits is_disabled, so it defaults to None ("leave unchanged"), and the app is created enabled regardless of the source config's flag. The PR description discloses this ("a cloned app deploys enabled ... out of scope"), but the fix is a two-line addition: update_config already accepts is_disabled: bool | None (client/configs.py:497, exercised by this PR's own FakeApi.update_config test double), and the sibling call two lines below (_sync_push_ops.py:454) already computes bool(local_data.get("is_disabled", False)). This is also a regression relative to the pre-PR create_config path, which did honor is_disabled. Worth folding into this PR rather than a follow-up, given how small the fix is.

[NB-2] src/keboola_agent_cli/services/sync_service.py:1822-1836 — ds_client can leak if a push aborts on ENCRYPTION_FAILED

ds_client (a DataScienceClient, which implements __enter__/__exit__ per data_science_client.py:71-76) is built at line 1697 and closed manually at lines 1835-1836. The except Exception as exc: ... raise at lines 1822-1832 re-raises a KeboolaApiError(ENCRYPTION_FAILED) before the Phase-A loop reaches that close call, so ds_client.close() is skipped entirely on that path (the surrounding with client: at line 1671 still closes the storage client correctly — only ds_client is unguarded). CONTRIBUTING.md's "Resource management" convention calls for a context manager on anything with __enter__/__exit__; wrapping the Phase-A loop in with ds_client (or a contextlib.nullcontext() fallback when it is None) would close this. The trigger is narrow (a data-app CREATE with a recorded type, plus another change in the same push failing encryption), but kbagent serve is long-running, so a leaked client accumulates rather than dying with the process.

[NB-3] src/keboola_agent_cli/services/data_app_service.py:144-206 — no cleanup if update_config fails after create_app succeeds

The established two-step DS-create pattern in this same file, DataAppService.create (lines 483-634), wraps create_app + update_config in try / except Exception: ds_client.delete_app(app_id) ... finally: ds_client.close(), specifically so that "a failed data-app create does not leak an empty deployment shell" (module docstring, lines 1-13). create_synced_data_app runs the identical two-call sequence with no such guard: if update_config (line 198) raises after create_app (line 175) already succeeded, the orphan DS app and its bare Storage config are left in the target project, and sync push's per-change error handling (_record_push_error) reports only that the change failed, with nothing to indicate a stray app was created. Reusing the same cleanup pattern would close this gap.

[NB-4] Live push/clone path unverified against a real Data Science API

The PR description states the only live verification was a read-only pull against project 4214; sync push/sync clone creating a data app through create_synced_data_app is covered only by mocked (FakeDs/MagicMock) unit tests. I independently confirmed the /apps list field shape (componentId, configId, type) that the pull side depends on by running a read-only data-app list against the same project 4214 referenced in the PR description — it matches the assumptions in sync_service.py:588-601 exactly. I did not attempt a live sync push/sync clone create myself: that project carries 2126 configs (too large for a quick reproduction), and creating a real data app is outside what a read-only review should do unprompted. The create_app → update_config sequence and the parameters.id repoint therefore remain unverified against the real Data Science API.

Nits

• [NIT-1] src/keboola_agent_cli/services/data_app_service.py:175-181 — create_synced_data_app sends the full (still source-project-pointing) configuration as the initial create_app body, then corrects parameters.id and re-sends the whole body via update_config. The sibling pattern (DataAppService.create, lines 485-491) instead sends a minimal initial_config shell first and builds the correct full body only once, on the update_config call. Not a bug — the DS service treats config as opaque storage on create, and the correction lands in the same call chain — but the file's two DS-create call sites now follow different shapes for the same two-step contract.

Verification log

• git rev-parse --abbrev-ref HEAD → fix/cli-8-data-app-type; git rev-parse HEAD → 78bdc925daeebfc44527e52d8b80967fe28f33c2 — matches the given branch and PR head exactly.
• gh auth status → authenticated.
• gh -R keboola/cli pr view 752 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → state OPEN, fix(sync): prefix matches the change type, +577/-11 across 7 files (4 source, 3 test), no plugins/ or docs/ files touched.
• Read CONTRIBUTING.md (Plugin synchronization map, File-size budgets, per-command checklist) and CLAUDE.md (## All CLI Commands, convention #17) in full; read plugins/kbagent/agents/keboola-expert.md in full (§1, §3 as required, plus the rest for the data-app gotcha precedents cited above).
• 3-layer greps (typer/click/formatter/console.print in services/; httpx/requests in the touched service files) on the diff and on the touched files directly → empty, no layer violation.
• Convention greps (magic numbers outside constants.py, raw error_code="..." literals, bare except:, print() in production code, token-in-log) on the diff → empty.
• uv run make check → FAILED at loc-check, exit 2: FAIL: services/data_app_service.py grew to 1679 code lines, past its grandfathered 1641. Every earlier gate passed clean: lint, format, ty (one pre-existing, unrelated unresolved-import warning in scripts/hatch_build.py, does not block per the project's own ty rules), SKILL.md freshness, version-sync, version-gate-check, command-sync-check ("OK: all 268 CLI commands are registered (OPERATION_REGISTRY) and documented (CLAUDE.md, context.py, commands-reference.md)" — confirms no new/removed/renamed command, so no OPERATION_REGISTRY/AGENT_CONTEXT gap here), endpoints-check, changelog-check, check-error-codes, check-sentinel-guards.
• uv run make test (the target loc-check blocked) → 1 failed, 6579 passed, 12 skipped in ~104s; the one failure is tests/test_check_file_size.py::TestRepoState::test_repo_passes_its_own_gate, the same file-size gate — confirming the PR's actual sync/data-app logic is otherwise clean and the file-size budget is the only red signal.
• uv run python scripts/check_file_size.py --report | grep data_app_service → 1679 2188 18% services: HARD >1500 against a grandfathered baseline of 1641 in scripts/file_size_baseline.json.
• Read sync_service.py, _sync_push_ops.py, data_app_service.py, sync/config_format.py, sync/diff_engine.py, data_science_client.py, and client/configs.py around every touched region: traced the configuration encrypt-then-branch ordering (secrets are encrypted before the DS-routed branch, so no plaintext leak), confirmed _IGNORED_KEYS = frozenset({"_keboola", "version"}) in diff_engine.py (backs the PR's hash-invisibility claim), confirmed update_config's is_disabled support, and located the established cleanup pattern in DataAppService.create.
• Ran a read-only kbagent --json data-app list --project <alias> against project 4214 (the project referenced in the PR description) using the repo's own dev CLI — live /apps records carry componentId/type/configId exactly as the new pull code assumes (21 apps returned, a mix of python-js/streamlit, matching the PR description's counts).
• Ran a read-only kbagent --json config list --project <alias> against the same project → 2126 configs, which is why a live sync pull/sync push reproduction of the full flow was not attempted (see NB-4).
• grep '"sync\.' permissions.py → sync.push/sync.pull/sync.clone already registered (write/read/write); no new commands are added by this PR, consistent with the clean command-sync-check above.

Open questions for the author

(none)

[soustruh]

Thanks for the review. I addressed all six points in e278263.

• B-1 (file-size gate): moved create_synced_data_app into a new services/_sync_data_app.py. data_app_service.py is back at its 1641 baseline and make loc-check passes.
• B-2 (gotchas.md): added an entry for the data-app-type behavior, with matching notes in keboola-expert.md §3 and the sync clone bullet in commands-reference.md, tagged (since vNEXT).
• NB-1 (is_disabled): the DS create path now passes it to update_config.
• NB-2 (ds_client leak): the client enters on the with line, so it closes on every exit from the push block, a mid-push ENCRYPTION_FAILED raise included.
• NB-3 (orphan cleanup): if update_config fails after create_app, the code deletes the DS record, so a failed create leaves no orphan app.
• NIT-1 (create body shape): the create call drops the stale parameters.id, so create_app never sees the source project's app id.

New unit tests cover NB-1, NB-3, and NIT-1. The FakeDs double closes on __exit__ like the real client, which covers NB-2. make check is green: 6582 tests, loc-check, and version-gate-check.

NB-4 (a live sync push / sync clone against a real Data Science API) is not done. It needs a real create in a target project, so it stays covered by the mocked unit tests plus the read-only live pull the description records. I will run a live create if a throwaway target is available.

[soustruh]

Update on NB-4 (a live sync push / sync clone against a real Data Science API): done, and it found two create-path bugs the mocked tests missed. Fixed in c26867a.

• POST /apps rejected the full Storage body with HTTP 422. It wants the create-shell shape (parameters.size / autoSuspendAfterSeconds / dataApp.slug + authorization), not runtime.backend.size. Fixed: build the shell, then update_config sends the full body.
• POST /apps also wants branchId=null on a production push, but the sync engine passed the numeric default branch id. Fixed: map the default branch to null for the DS create, and keep a dev branch id.

Live check: a sync push of a python-js app to project 4214 created a DS record with type=python-js. I then deleted the test app. The unit tests now assert the shell shape and branchId=null.

One known limit: a session-token target still cannot create a data app, because the DS API rejects session tokens. That is out of scope here, and session support for this operation is a separate ticket.

[keboola-pr-reviewer-bot]

Verdict: auto_approve (risk 2/5) · profile keboola-mcp-server

Well-tested, backward-compatible sync bug fix carrying a data app's runtime type through pull/push/clone.

Concerns:

• src/keboola_agent_cli/services/_sync_data_app.py: Bare except Exception around update_config; intentional orphan-cleanup, logged, non-blocking coaching note

[zajca]

Reviewed c26867a against base main @ 99a000b (true base→head diff, 9 files, +711/−13). Ran pytest tests/ -k "sync or data_app": 902 passed, 29 skipped (test_serve_* excluded — fastapi missing in the venv).

The direction is right and the DS-record/Storage-config split is documented well. Three things I'd like addressed before merge — the first one undoes the fix on the next push.

---

1. parameters.id back-pointer is never written back to the local file, so the next sync push overwrites the correct remote id with the stale source one

services/_sync_data_app.py:96-98 sets parameters.id to the newly created app id in the body that goes to the remote. But writeback_after_push (_sync_push_ops.py:466 → _sync_writeback.py:236-257 → apply_encrypted_to_local) only copies encrypted #-keys back into _config.yml. The local file keeps the source app's parameters.id.

So after sync clone the remote is correct and the local tree is not. Any later edit to that config goes through push_update, which sends local_config_to_api(local_data) including the stale parameters.id — silently reverting the back-pointer that data_app_service.py:465 calls "writeup §5: required". Phases C/D don't help: _sync_bindings.py:86 and :390 filter on keboola.variables / keboola.flow only.

The test suite already shows the asymmetry: tests/test_sync_data_app_type.py:256 asserts the new id in the PUT, while the local fixture keeps "99999" (:167).

Suggested fix: have create_synced_data_app return the new app id to the caller (or have push_create persist it), and write parameters.id into pristine_data before writeback_after_push. A test asserting the on-disk parameters.id after a create would lock this in.

2. A Data Science failure during sync pull silently deletes an already-recorded data_app_type from disk

services/sync_service.py:589-604 degrades to an empty map with only a logger.warning. sync_service.py:762-771 then rewrites _config.yml without data_app_type (the write is unconditional for an unmodified config) and re-stamps pull_hash.

One ordinary pull during a DS outage — or with a token lacking DS scope — therefore strips the type from every data-app config in the tree. Because config_hash ignores _keboola (sync/diff_engine.py:21) and pull's result envelope has no warnings channel (unlike push, sync_service.py:1949), neither sync diff nor --json shows anything. A later clone then deploys under the platform default — exactly the bug this PR fixes, reintroduced invisibly.

This is a different class from the neighbouring except Exception blocks for storage/jobs: those only skip auxiliary metadata files, this one removes a value from a tracked config file. Please either preserve the existing _keboola.data_app_type when the DS lookup did not run/succeed, or surface a warning on the pull envelope (ideally both).

3. Orphan app left when POST /apps returns id without configId — contradicts the docstring

services/_sync_data_app.py:88-94 raises before the try/except that calls delete_app. With an id present but configId missing, the DS record and its bare Storage config stay behind in the target, while the docstring (:56-58) promises "a failed sync create leaves no orphan app in the target". tests/test_sync_data_app_type.py:395-410 covers the raise but does not assert cleanup.

---

Verified as correct

• _keboola is in diff_engine._IGNORED_KEYS (:21), so the claim of zero sync diff noise on existing trees holds.
• The create shell matches DataAppService.create (data_app_service.py:420-427); runtime.backend.size / authorization survive the _configuration_extra round-trip.
• manifest.branches[0] as the default branch is the established convention repo-wide; mapping production to branchId=null matches data-app create.
• The new SyncService.__init__ signature is backwards compatible — all 20 call sites use kwargs.
• The componentId == keboola.data-apps filter does prevent sandbox/workspace records from landing on an unrelated config; the DS client is built only when the changeset creates a data app, and with client, ds_context closes both on a mid-push raise.
• Docs correctly use the vNEXT placeholder, with no version bump and no changelog.py entry.

One thing worth confirming (I couldn't verify without a live stack)

A data app's slug forms its hostname, so it is presumably unique per project. push_create for a fork-by-copy CREATE within the same project now sends POST /apps with the same slug as the source. If the API rejects that, it is a behaviour change from the previous create_config path — worth a live check.