Skip to content

feat(sync): run the runtime-safety script guard on the push path (follow-up to #686) - #696

Merged
padak merged 1 commit into
mainfrom
feat/issue-686-followup-push-runtime-guard
Aug 25, 2026
Merged

feat(sync): run the runtime-safety script guard on the push path (follow-up to #686)#696
padak merged 1 commit into
mainfrom
feat/issue-686-followup-push-runtime-guard

Conversation

@padak

@padak padak commented Aug 25, 2026

Copy link
Copy Markdown
Member

Stacked on #694 (claude/issue-686-pr-draft-867ec7) — the diff below is only this PR's commit. Follow-up to #694 / issue #686, as its "Follow-up" note announced. Retarget the base to main once #694 merges.

Why

normalize_blocks_codes_script is the runtime-safety guard for
parameters.blocks[].codes[].script: it turns a string into an array (#245) and
re-splits an element that packs several ;-separated statements (#274). The
Storage API accepts both broken shapes silently; the Keboola runtime then
fails the job with Expected "array", but got "string" or
Actual statement count N did not match the desired statement count 1.

It has run on config update since 0.28.0 and on transformation edit/create
since 0.30.8. sync push — the GitOps deploy route — was the one path to the
Storage API without it.

Belt and braces, not a fix

After #694's parts 2+3 the guard is a no-op on this path by construction:
merge_code_files rebuilds parameters.blocks from transform.sql through the
single canonical producer (canonical_sql_script), and the boundary markers
make that round-trip lossless. Wiring it in is a regression backstop — if a
future change to code extraction or the canonical producer regresses the shape,
the push is corrected instead of deploying a job that crashes at runtime.

Its semantics are untouched; it is called, not modified.

It is also not purely theoretical today. One shape bypasses code extraction
entirely: a hand-authored _config.yml carrying parameters.blocks inline
with no companion transform.sql. _merge_sql_transformation returns early
when the file is absent, so those parameters reach the API verbatim. Two of the
new tests fail on origin/claude/issue-686-pr-draft-867ec7 for exactly that
reason (the string arrives at the API as a string).

What changed

  • New guard_script_shape() in services/_sync_push_ops.py — wraps the helper
    unchanged and shapes its records into push-envelope warnings.
  • Called in push_create and push_update, after merge_code_files()
    • local_config_to_api() produce the API configuration body, before
      encryption and send.
  • Also called in the Phase C variables backfill (_sync_bindings. _apply_variable_binding): it re-PUTs the whole configuration, so it is the
    last write a freshly-created transformation receives — an unguarded body there
    would undo what push_create had just fixed. Phase D (flow task remap) is not
    guarded: flows carry no blocks/codes, so the call would be dead code.
  • Rows are deliberately not guarded. Code extraction is config-level only —
    merge_code_files is never called for a row, and SQL/Python transformations
    are row-less components whose blocks live in the root config's parameters.
    The sibling CLI path (config row-create / config row-update) is likewise
    unguarded, so guarding here would diverge from the convention this PR is
    mirroring. Say the word if you want it symmetric anyway.

What is surfaced, where

config update's convention is a dedicated normalizations key on its
envelope. A push envelope spans many configs, so each record instead carries its
own identity and joins the existing warnings[] list added by #694:

{
  "change_type": "script_normalization",
  "component_id": "keboola.snowflake-transformation",
  "config_id": "cfg-sql",
  "config_path": "transformation/keboola.snowflake-transformation/raw-data-processing",
  "path": "parameters.blocks[0].codes[0].script",
  "action": "sql_split",
  "before_type": "str", "after_type": "list", "after_length": 2,
  "message": "Normalized …/cfg-sql … before the write (…): the local files held a script shape the Keboola runtime rejects. Run 'kbagent sync pull' …"
}
  • --json: structural, under warnings on the push envelope.
  • human mode: printed by the existing push-warning loop in
    commands/sync.py — no change needed there, which is the point of reusing the
    channel. Both are pinned by tests.

warnings[] element type widened from dict[str, str] to dict[str, Any]
(after_length is an int).

Tests — tests/test_sync_push_script_guard.py (11)

TDD: the three integration tests below were written first and failed on the
parent branch (the crashing shape reached the API verbatim).

  • string script on UPDATE → sent as an array, record surfaced with the right
    action / after_length / identity;
  • list element packing two statements on UPDATE → re-split (sql_resplit);
  • string script on CREATE (hand-authored, never-pulled config dir);
  • no-op path: the ordinary pull → edit transform.sql → push flow produces
    zero records, sends the canonical array, and still diffs in sync;
  • the same with semicolon-less elements (["SELECT 1", "SELECT 2"]);
  • a non-transformation component is untouched;
  • three direct unit tests of guard_script_shape (record shaping, silence on a
    canonical body, warnings=None still fixes the body);
  • CLI human-mode and --json surfacing.

make check green: 6200 passed, 12 skipped; ruff / ty / version-gates /
command-sync / changelog / error-codes / sentinel-guards / loc-check all clean.

Release notes

No version bump, no changelog.py entry (per the release process). Docs tagged
(since vNEXT) in gotchas.md and sync-workflow.md.

@padak
padak marked this pull request as ready for review August 25, 2026 13:01

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Base automatically changed from claude/issue-686-pr-draft-867ec7 to main August 25, 2026 13:18
`normalize_blocks_codes_script` -- the #245/#274 guard that turns a
`parameters.blocks[].codes[].script` string into an array and re-splits an
element packing several `;`-separated statements -- ran on `config update`
(0.28.0) and `transformation edit/create` (0.30.8) but never on `sync push`,
the one remaining deploy route to the Storage API.

After #686 parts 2+3 the guard is a no-op on the GitOps path by construction
(`merge_code_files` rebuilds the blocks through the single canonical producer),
so this is a regression backstop rather than a correctness fix. It does still
cover the shape that bypasses code extraction entirely: a hand-authored
`_config.yml` carrying `parameters.blocks` inline with no companion
`transform.sql`, which `merge_code_files` passes through verbatim -- a shape
the Storage API accepts and the job runtime rejects.

- new `guard_script_shape()` in `_sync_push_ops.py` wraps the helper unchanged
  and shapes its records into push-envelope warnings;
- called in `push_create` / `push_update` after `merge_code_files` +
  `local_config_to_api`, before encryption and send;
- called in the Phase C variables backfill too: it re-PUTs the WHOLE body, so
  it is the last write a freshly-created transformation receives;
- rows are deliberately NOT guarded: code extraction is config-level only
  (`merge_code_files` is never called for a row) and the sibling
  `config row-create` / `row-update` path is likewise unguarded;
- records surface as `warnings[]` entries with `change_type:
  "script_normalization"` (`path` / `action` / `after_length` kept), so human
  mode prints them through the existing push-warning loop and `--json` carries
  them structurally. `config update` keeps its dedicated `normalizations` key;
  a push envelope spans many configs, so each record carries its own identity.

`warnings[]` element type widened to `dict[str, Any]` -- the records carry a
non-string `after_length`.

Docs: `(since vNEXT)` notes in gotchas.md and sync-workflow.md.
@padak
padak force-pushed the feat/issue-686-followup-push-runtime-guard branch from 0f1bd55 to 4b49eb5 Compare August 25, 2026 13:21
@padak
padak merged commit 9e687aa into main Aug 25, 2026
3 of 6 checks passed
@padak
padak deleted the feat/issue-686-followup-push-runtime-guard branch August 25, 2026 13:21
@padak padak mentioned this pull request Aug 25, 2026
13 tasks
padak added a commit that referenced this pull request Aug 25, 2026
Bump pyproject to 0.91.0, add the changelog entry covering every PR merged since v0.90.1 (#627, #681, #691, #692, #693, #694, #695, #696, #697, #698), resolve all 54 vNEXT version-gate placeholders, and run version-sync + skill-gen.

No web/frontend changes in this batch, so no whatsnew.ts entry.
padak added a commit that referenced this pull request Aug 25, 2026
CONTRIBUTING.md release step 4 keeps version tags off headings: resolving the
placeholder changes the generated anchor slug and breaks inbound links. PR #697
did this pass, but #694 and #696 landed their headings around the same time and
fell outside its grep.

- gotchas.md: two headings, tag moved to the section's first body line
- sync-workflow.md: heading reworded to 'Migrating a legacy sync tree', with
  the version stated on a body line

No inbound links reference the affected anchors (verified by grep).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant