feat(sync): clone composite + flow-task configId remap (#426) - #431
Merged
Conversation
Add `kbagent sync clone` (and `SyncService.clone_project`) -- a first-class composite that builds a new project by cloning a reference synced tree and parameterizing it, instead of every consumer re-implementing the copy + id-rewrite + push surgery by hand. - New push Phase D (`_resolve_flow_task_bindings`): after the Phase-C variable backfill, remap `keboola.flow` task configIds (configuration.tasks[].task. configId, job tasks only) from source ids to the ULIDs created this push, via the existing created_id_map. Generic -- benefits any fresh-create push, not just clone. PUTs the corrected flow, rewrites local, refreshes hashes. - sync/clone.py: pure, client-free override helpers -- bucket_map (rewrite the bucket prefix of storage input/output table refs), variable_values (override keboola.variables row values), instance_rename (rename config-path prefixes on disk + in the manifest), plus copy_reference_tree + repoint_manifest_project. - SyncService.clone_project: copy -> re-point manifest -> apply overrides -> fresh-target guard (every config must diff as 'added', else refuse) -> push. Cloning into a fresh target needs NO id surgery: the reference ids don't exist there, so the diff CREATEs everything and created_id_map (keyed by reference id) drives the Phase-C/D link remaps. Idempotent: a re-run with an existing target-dir skips copy/overrides and just pushes -> no_changes / created: 0. - CloneResult typed model (exported), embeds SyncPushResult. Registers the sync.clone write operation; regenerates SKILL.md. Docs synced: CLAUDE.md command list, AGENT_CONTEXT, commands-reference, gotchas (since v0.63.0), sync-workflow, library-workflow. Tests: pure override helpers, Phase-D flow remap, clone orchestration (overrides/guard/idempotency/ dry-run), CLI, and a `sync clone --dry-run` E2E step. make check green (4089).
| raise ConfigError(f"Cannot parse override file {path}: {exc}") from exc | ||
| if not isinstance(data, dict): | ||
| raise ConfigError(f"Override file {path} must contain a JSON/YAML object (mapping).") | ||
| return {str(key): str(value) for key, value in data.items()} |
There was a problem hiding this comment.
🟡 _load_override_file silently converts YAML null values to the string "None", corrupting override maps
When a YAML override file contains a null value (e.g., in.c-old: with no value, or in.c-old: null), yaml.safe_load returns Python None. The comprehension {str(key): str(value) for key, value in data.items()} at line 69 converts None to the literal string "None". For a --bucket-map file this would silently rewrite table references to use bucket id "None" (e.g., None.customers); for --variable-values it would set variable values to "None". This is a well-known YAML foot-gun (bare values like yes, no, null, and missing values all get type-coerced by safe_load) and should be caught with a validation check before coercion.
Suggested change
| return {str(key): str(value) for key, value in data.items()} | |
| return {str(key): str(value) for key, value in data.items() if value is not None} |
Was this helpful? React with 👍 or 👎 to provide feedback.
padak
added a commit
that referenced
this pull request
Jun 17, 2026
#426/#427/#428) (#432) * feat(lib): typed SDK return models + py.typed marker (#428) (#429) Typed pydantic return models (JobResult/QueryResult/UploadTableResult/SyncPushResult/ConfigDetailResult) + py.typed marker + typed facade wrappers. Part 1/3 of the SDK-hardening collector. * feat(job): client-side idempotency key for run_job (#427) (#430) Optional --idempotency-key on job run + Client.run_job: client-side dedup store (Queue API has no server token, verified vs live spec). Part 2/3 of the SDK-hardening collector. * feat(sync): clone composite + flow-task configId remap (#426) (#431) kbagent sync clone + SyncService.clone_project: copy a reference tree, apply bucket/variable/instance overrides, push fresh with flow-task + variable-link remap (new push Phase D). Part 3/3 of the SDK-hardening collector. * fix(review): address #432 review findings (Devin + kbagent-pr-reviewer) - context.py AGENT_CONTEXT (Devin BUG): the `sync clone` block was inserted between the `sync push` one-liner and its multi-paragraph continuation, so push semantics (encryption fail-closed, fresh-CREATE, --branch) mis-read as clone properties. Move the clone block AFTER the full push description. - keboola-expert.md (NB-1): add a `sync clone` row to the §2 tool matrix so the agent recommends the composite for "provision a new project from a reference", not the manual pull+edit+push flow. - library-workflow.md (NB-2): clarify CloneResult DOCUMENTS the dict shape that SyncService.clone_project returns (the service returns a plain dict; wrap via model_validate) -- it is not returned as a typed instance. - test_result_models.py (NB-2): add CloneResult contract tests (embedded SyncPushResult, ok property, dry_run-without-push) + cover it in the base loop. - test_e2e.py (NB-3): document why the clone E2E step is --dry-run only (no fresh second project in the single-project E2E harness; push path is unit-covered). - job_idempotency_store.py (NIT-2): comment that force_rerun intentionally bypasses the collision guard (don't hoist the check out of the branch). - sync-workflow.md: note that `sync clone --dry-run` still writes --target-dir. NIT-1 (formatter: Any) intentionally NOT applied: all sibling _format_* helpers in sync.py use `Any` -- matching the established pattern. NB-4 (sync_service.py LOC split) tracked as a follow-up tech-debt task. make check green (4092).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 3 of 3 of the SDK-hardening collector branch (#426/#427/#428). Builds on the typed
SyncPushResult/CloneResultfrom #428.What
Closes #426. A first-class
kbagent sync clone(andSyncService.clone_project→ typedCloneResult) that builds a new project by cloning a reference synced tree and parameterizing it — replacing the hand-rolled, untested copy + id-rewrite + push surgery every consumer (e.g. the FIIA Scaffold Kit) currently re-invents.How
_resolve_flow_task_bindings): after the Phase-C variable backfill, remapkeboola.flowtaskconfigIds (configuration.tasks[].task.configId, job tasks only) from source ids → the ULIDs created this push, via the existingcreated_id_map. Generic — benefits any fresh-create push (scaffold included), not just clone. Mirrors the Phase-C PUT + local-rewrite + hash-refresh pattern. The push result gainsflow_task_remaps.sync/clone.py— pure, client-free override helpers:bucket_map(rewrite the bucket prefix of storage inputsource/ outputdestination),variable_values(overridekeboola.variablesrow values),instance_rename(rename config-path prefixes on disk + in the manifest), pluscopy_reference_tree+repoint_manifest_project.SyncService.clone_project— copy → re-point manifest at the target → apply overrides → fresh-target guard (every config must diff asadded, else refuse rather than UPDATE a stranger's config) → push.Key insight: no placeholder surgery needed
Cloning into a fresh target needs no id reset. The reference's config ids don't exist in the target remote, so the diff classifies every config as
addedand push assigns new ULIDs — and becausecreated_id_mapis keyed by the reference id (the manifest entry's id before writeback), the Phase-C variable links and the Phase-D flow task configIds remap reference→ULID automatically. Idempotent: a re-run with an existing--target-dirskips copy/overrides and just pushes →no_changes/created: 0.Surface
sync clone --source DIR --target ALIAS --target-dir DIR [--bucket-map FILE] [--variable-values FILE] [--instance-rename FILE] [--dry-run] [--branch ID](override files are JSON/YAML maps).CloneResultexported from the package root (embedsSyncPushResult).sync.clonewrite operation; regenerated SKILL.md.Docs
CLAUDE.md command list,
AGENT_CONTEXT,commands-reference.md,gotchas.md(since v0.63.0),sync-workflow.md(clone section),library-workflow.md.Tests
tests/test_sync_clone.py: pure override helpers (bucket/variable/instance), Phase-D flow remap via the service (PUT + local rewrite + hash refresh + no-op-on-no-match), clone orchestration (overrides applied, manifest re-pointed, fresh-target guard, idempotent re-run, dry-run, unsynced-source error), and the CLI command. Plus a non-mutatingsync clone --dry-runE2E step inTestE2ESyncWorkflow.make checkgreen (4089 passed).