sync push stamps pull_config_hash from disk while diff compares an API-derived hash — every pushed config stays "REMOTE MODIFIED" forever (mirror of #466)

[DasaDama]

Summary

sync push stamps pull_config_hash with a hash computed from the files on disk, while sync pull and sync diff compute the remote-side hash from the API response. For any config where the local↔API round-trip is not hash-stable, those two producers disagree — so after a successful sync push the config is reported ~ REMOTE MODIFIED by every subsequent sync diff, forever, even though the remote and the working tree are byte-identical. Only a real sync pull clears it.

The stored hash exists precisely to avoid that round-trip — the diff path says so itself:

# For files unchanged since pull, use the stored pull_config_hash
# directly (avoids lossy code extraction roundtrip).

…but push fills that field with a value produced by the lossy round-trip.

Relation to #466 (please read together)

This is the mirror image of #466 and it survived the v0.72.0 fixes. #466 was sync diff/status falsely reporting "in sync" (false negative) plus reconcile ergonomics; this is sync diff falsely reporting drift (false positive), and the root cause is on the push side — nothing in the #466 / #467 / #482 / #497 fixes touches it. The --theirs reconcile path shipped for #466 does clear the symptom (any real pull does), but it cannot prevent it: the next deploy re-creates it.

Net effect for us is the same conclusion as #466's impact section, from the opposite direction: sync diff still cannot be trusted as a local-vs-production audit — now because it reports drift that does not exist.

Impact — why this is urgent for us

We keep a git repo in lock-step with production: each project is a sync working tree, and a production deploy is sync push after a merged PR (~20 projects on one stack).

1. Every deploy now needs a second, content-free PR. The documented "refresh the manifest baseline after deploy" chore cannot work, because committing the manifest that push just wrote stores the wrong hash form. To get a truthful baseline into git we have to open a separate PR whose entire diff is manifest.json — after every single deploy. That is what pushed us to file this.
2. Real drift hides in the noise. Our repo currently carries 18 phantom REMOTE MODIFIED configs against 2 real UI changes. We could only separate them by pulling all of production into a scratch tree and comparing file by file — exactly the kind of manual audit kbagent sync: cannot reliably reconcile a drifted tree with production; sync diff/status report false "in sync" #466 was supposed to make unnecessary.
3. It is self-inflicted by the tool's own happy path: the more disciplined you are about deploying through sync push, the more phantom drift you accumulate.

Root cause (paths on main, v0.90.1)

step	file:line	pull_config_hash computed from
sync pull	src/keboola_agent_cli/services/sync_service.py:787-793	config_hash(api_config_to_local(component_id, cfg, config_id)) — the API response
sync push (modified)	sync_service.py:1710 → _compute_config_hashes (:1908-1930)	self._read_config_file(config_dir) + merge_code_files(...) — the files on disk
sync diff	sync_service.py:1251 builds the remote side with api_config_to_local(...), then compares it against the stored pull_config_hash	mixed producers ⇒ mismatch

The same asymmetry is in the create path (sync_service.py:1652) and in the row push path (services/_sync_push_ops.py:239).

Every affected config in our data has at least one code whose API body holds parameters.blocks[].codes[].script as an array of several statements, which transform.sql cannot represent (there are no statement boundaries in the file, and _config.yml carries no blocks/codes/script at all), so merge_code_files cannot reconstruct a hash-identical body. We did not isolate the trigger further than that.

Evidence

Observed on kbagent v0.59.0, one stack, 21 projects, 244 configs. (Still on 0.59.0 because auto-update cannot cross the #424 package rename, as you noted on #466 — we will reinstall and re-verify. All three code paths above are unchanged on main @ v0.90.1, which is why we are filing now rather than waiting.)

Committed baseline vs a fresh sync pull --all-projects of the same production state, compared field by field:

• 224 configs — baseline identical.
• 18 configs — pull_config_hash differs and nothing else: pull_hash and pull_extra_hashes (transform.sql, _description.md) match exactly, and the pulled files are byte-identical to the tracked ones (0 differing files across 17 project trees, 11–139 files each).
• 2 configs — genuinely changed in the UI (file hashes differ too). These are the true positives the noise was hiding.

The discriminator is who wrote the remote's current version (config detail → currentVersion.creatorToken.description), and it was exceptionless:

remote version last written by	stored pull_config_hash
kbagent-cli [...] — i.e. a sync push	mismatches (18/18)
a plain user token — i.e. the UI	matches

Ruled out as the discriminator: multi-statement codes per se. Several configs whose codes hold up to 39 statements each compare clean — because their baseline was written by a pull, not a push.

Finally, sync diff run against the freshly pulled tree reports all 21 projects in sync, confirming the API-derived form is the one diff accepts.

Repro

1. In a sync working tree, pick a keboola.snowflake-transformation whose API body has a code with more than one statement in script.
2. Change anything in it locally and kbagent sync push --project <alias>. Push succeeds.
3. kbagent sync diff --project <alias> → ~ REMOTE MODIFIED … parameters.blocks[0].codes changed, with no local edits and nothing changed on the remote since. It never goes away.
4. kbagent sync pull --project <alias> --no-jobs --no-storage → now in sync, and the only thing that changed on disk is manifest.json (pull_config_hash). No config file is touched, which is the proof that step 3 was phantom.

Suggested fix

Make push stamp the baseline with the same producer pull/diff use: hash the config as the API returns it after the write (the update response body, or a re-fetch), via config_hash(api_config_to_local(...)), instead of _compute_config_hashes(config_dir, ...). Applies to the config update path, the create path and the row path.

If that is not desirable, then a push should not stamp pull_config_hash at all — leaving it stale is at least visibly stale, whereas today it is written with a value that can never match.

A regression test would be: push a config with a multi-statement code, then assert sync diff reports it in sync.

[DasaDama]

Update: reinstalled via install.sh as advised on #466 — now on v0.90.1, and this reproduces unchanged.

$ kbagent --version
kbagent v0.90.1
$ kbagent sync diff --project <l1-stack>
Remote changes (run 'sync pull' to fetch):
  ~ REMOTE MODIFIED keboola.snowflake-transformation/.../raw-data-processing
      parameters.blocks[0].codes changed

Same 16 sibling projects, same single config each, files still byte-identical to the tracked ones — nothing in the working tree or on the remote changed in between, and the remote version is still the one written by the sync push deploy weeks ago. The counts in the report above (224 identical / 18 pull_config_hash-only / 2 real) were measured on v0.59.0; the symptom and the three code paths are unchanged on v0.90.1.

One data point that came out of the upgrade and is worth having here: v0.90.1 legitimately reports a lot more remote drift than v0.59.0 in the same trees, because it tracks fields the old version did not (is_disabled on configs and rows, per #467). That makes the phantom codes changed entries actively harmful — they sit in the same output as real, newly-detectable drift, and there is no way to tell them apart without pulling production into a scratch tree and diffing file by file.

[padak]

Reproduced on main @ fc7ae7db (v0.90.1). Thanks — the report is accurate and the three-file root-cause table is right. Two corrections/additions from digging in:

• the root cause is one level below where you placed it: the two hashes disagree because two functions hold contradictory definitions of what script[] looks like, so unifying the hash producers alone treats the symptom;
• the trigger is not "the API body holds script as an array of several statements" — it is semicolons in transform.sql. That flips which of your configs are affected, and it explains your 39-statement counter-examples without needing the pull-vs-push discriminator;
• and there is a second, silent failure mode in the same round-trip that your audit could not have seen, because it reports in sync. It changes production content.

Root cause: two contradictory normalizations of script[]

side	function	contract
pull + the remote side of diff	_normalize_scripts — sync/config_format.py:18	collapses each code's script into exactly one joined string
push + the local side of diff	_lines_to_script — sync/code_extraction.py:23	for SQL components, split_statements() → one element per statement

They are mutually exclusive whenever the SQL text has ≥2 statements, so the hashes cannot match — and never converge, no matter how many times you push: the API side always collapses to 1, the disk side always splits to N.

_normalize_scripts still justifies itself with:

This normalizes to single-string-per-code-block format so that local merge (which produces single-string via _lines_to_script) and remote data compare identically.

That parenthesis stopped being true in April 2026. Commit 17e23a02 (PR #120, "preserve multi-element script[] arrays in sync pull/push roundtrip", closing #119 — Snowflake MULTI_STATEMENT_COUNT=1) rewrote _lines_to_script to split on semicolons and did not touch config_format.py. _normalize_scripts predates it (e42a9ee3, 2026-03-25). So this has been latent since 0.2x, not since 0.72.0 — consistent with you seeing it on v0.59.0 and with all three paths being unchanged on main.

Refined trigger

Driving the real pull → push → diff path (api_config_to_local → extract_code_files → merge_code_files → config_hash):

API script[]	API elems	push sends	remote content	sync diff
["SELECT 1;", "SELECT 2;"]	2	2	same	PHANTOM
["SELECT 1;", "SELECT 2"] (last without ;)	2	2	same	PHANTOM
["SELECT 'a;b';", "SELECT 2;"] (; in a literal)	2	2	same	PHANTOM
["SELECT 1;\nSELECT 2;"] (one elem, 2 stmts)	1	2	CHANGED	PHANTOM
["SELECT 1"]	1	1	same	in sync
["SELECT 1", "SELECT 2"] (no ; anywhere)	2	1	CHANGED	in sync
["SELECT 1", "SELECT 2", "SELECT 3"]	3	1	CHANGED	in sync

What decides is how many statements split_statements() finds in transform.sql, i.e. semicolons — not the API array length. Note rows 4 and 6: a single-element API body can be affected, and a three-element one can be clean.

Your discriminator (currentVersion.creatorToken.description) being exceptionless is still exactly right, but it is a consequence rather than the cause: a pull-written baseline matches diff's remote side by construction, because both are the same config_hash(api_config_to_local(...)) call. So a pull baseline masks the disagreement regardless of statement count — which is why your 39-statement configs compared clean.

Second failure mode (not in the report): push can silently rewrite production

Rows 6 and 7 above. When the API's elements carry no trailing semicolons, transform.sql has no statement boundaries left to recover, so merge_code_files returns one element holding the whole text — and push writes that to the remote. Two statements become one, with no separator between them.

That is the MULTI_STATEMENT_COUNT=1 shape #119/#120 set out to eliminate; #120 fixed it only for the semicolon-delimited case. And it is invisible: because push then stamps the collapsed form and the API returns the collapsed form, sync diff says in sync. It is the mirror of the phantom drift — same round-trip, opposite symptom, and the quiet one.

Two things make it worse than it looks:

1. normalize_blocks_codes_script() (sync/code_extraction.py:106) is the write-side runtime-safety guard added for config update: re-split script[] elements that contain multiple statements (gap beyond #245) #274/fix(0.30.8): close issue #274 -- per-element sql_resplit closes ODBC statement-count crash #275 precisely to stop this class of crash. It is wired into config update (config_service.py:752) and transformation edit/create (transformation_service.py:213,282) — and not into sync push (zero references in sync_service.py / _sync_push_ops.py). The GitOps path is the one deploy route without that guard.
2. Even if it were wired in, it would not help here: that pass re-splits elements containing multiple ;-separated statements, and this case has no semicolons to split on.

I have not measured how common no-trailing-semicolon elements are in your 21 projects — that is worth checking on your side before deciding how urgent this half is. Anything the UI's own splitter wrote will normally end in ;; the elements that do not tend to come from older UIs, hand-edited configs, or keboola-as-code round-trips (the same provenances #274 called out).

Repro (no project needed)

import json, tempfile
from pathlib import Path
from keboola_agent_cli.sync.code_extraction import extract_code_files, merge_code_files
from keboola_agent_cli.sync.config_format import api_config_to_local
from keboola_agent_cli.sync.diff_engine import config_hash

COMP = "keboola.snowflake-transformation"
def api(script):
    return {"id": "1", "name": "c", "description": "", "configuration":
            {"parameters": {"blocks": [{"name": "B", "codes": [{"name": "C", "script": list(script)}]}]}}}

for script in (["SELECT 1;", "SELECT 2;"], ["SELECT 1", "SELECT 2"]):
    d = Path(tempfile.mkdtemp())
    local = api_config_to_local(COMP, api(script), "1")       # pull
    extract_code_files(COMP, local, d)
    pushed = json.loads(json.dumps(local))
    merge_code_files(COMP, pushed, d)                          # push
    push_h = config_hash(pushed)
    sent = pushed["parameters"]["blocks"][0]["codes"][0]["script"]
    remote_h = config_hash(api_config_to_local(COMP, api(sent), "1"))   # diff, remote side
    print(script, "-> sent", sent,
          "| content", "SAME" if sent == script else "CHANGED",
          "| diff", "in sync" if remote_h == push_h else "PHANTOM")

['SELECT 1;', 'SELECT 2;'] -> sent ['SELECT 1;', 'SELECT 2;'] | content SAME    | diff PHANTOM
['SELECT 1', 'SELECT 2']   -> sent ['SELECT 1\nSELECT 2']     | content CHANGED | diff in sync

How to fix

Your suggestion — have push stamp config_hash(api_config_to_local(component_id, <API response>, config_id)) — is feasible as written: update_config already returns the updated configuration (client/configs.py:526), it is simply discarded today (_sync_push_ops.py:361 calls it without binding the result). Same for the create and row paths. So no re-fetch is needed, just threading the response back to the manifest stamp.

But on its own it makes diff agree by making both sides speak the collapsed, one-element form — leaving the two contradictory definitions in place, and leaving the silent rewrite untouched. It would fix your 18 phantoms and nothing else.

The fuller fix is to pick one canonical shape for script[] and make both functions produce it. The runtime already picked: each array element is one executable statement — that is the whole premise of #119/#120/#274. Which points at making _normalize_scripts split like _lines_to_script does, rather than the reverse, so the API side stops collapsing. That also closes the silent rewrite, because statement boundaries would survive the transform.sql round-trip instead of being re-derived from semicolons alone (this needs the file format to carry boundaries the splitter cannot infer — the part that actually needs designing).

Two things a fix has to handle either way, worth flagging before anyone starts:

• Migration. Every manifest in the wild carries push-stamped hashes in the old form. Changing the producer makes the first diff after the upgrade re-classify those configs — so the change needs either a one-shot re-stamp path or an explicit "run sync pull once after upgrading" note. Landing it silently would hand you a third flavour of phantom drift, which is the opposite of the point.
• A regression test in both directions — your suggested one (push a multi-statement config, assert diff reports in sync), plus its mirror: assert push sends the same statement count it pulled, which is the one that pins the silent case.

Marking this a bug and putting it in the queue. The --theirs note in your #466 follow-up is right and worth repeating for anyone landing here: it clears the symptom, and the next deploy re-creates it — so it is a workaround, not a fix.

One aside on your environment: since you are still on v0.59.0, everything above is verified against main @ v0.90.1, not against what you are running. The behaviour is identical (all three paths unchanged), but the reinstall is still worth doing before you re-measure your 18/2 split, so the numbers you get back are from the same code this is being fixed in.

[padak]

Thank you @DasaDama — this one earned its keep well beyond the symptom you reported.

Chasing the phantom ~ REMOTE MODIFIED turned up a second, worse defect hiding behind the same root cause: when the API's script[] elements carry no trailing semicolons, transform.sql had no statement boundaries left to recover, so sync push silently collapsed N statements into one — the MULTI_STATEMENT_COUNT=1 shape that fails at runtime — while sync diff reported in sync. That was quietly rewriting production transformations, and nothing would have surfaced it without your report.

Fixed in #694 (+ #696 wiring the runtime-safety script guard onto the push path as a regression backstop). Push now stamps the baseline from the API response, there is one canonical script[] shape, and transform.sql gains /* ===== STATEMENT ===== */ markers when semicolons cannot recover the boundaries.

Merged to main and queued for v0.91.0. One sync pull per project migrates an existing tree.