Skip to content

feat(0.31.1): config new --push for one-shot remote create - #278

Closed
ottomansky wants to merge 1 commit into
keboola:mainfrom
ottomansky:feat/config-create
Closed

feat(0.31.1): config new --push for one-shot remote create#278
ottomansky wants to merge 1 commit into
keboola:mainfrom
ottomansky:feat/config-create

Conversation

@ottomansky

Copy link
Copy Markdown
Contributor

Summary

Extends kbagent config new with a --push flag that POSTs to the Storage API in one CLI call. Closes the F2 gap from kbagent-feature-gaps.md — previously the only "FIIA HTTP call → kbagent CLI" mapping without a first-class command.

  • Without --push: byte-for-byte preserved (verified by unchanged TestConfigNew suite).
  • With --push: scaffolds + POSTs to /v2/storage/components/{cid}/configs. Returns the new config ID.
  • FIIA-style one-shot: --push --no-files skips the filesystem entirely.
kbagent config new --component-id keboola.ex-http --name "API ingest" \
  --project prod --push --no-files                 # one-shot, no files
kbagent config new --component-id keboola.python-transformation-v2 \
  --name "T1" --project prod --push --output-dir ./scratch   # scaffold + POST
kbagent config new --component-id keboola.ex-http --name "smoke" \
  --project prod --push --no-files --dry-run       # preview + validation

Why --push and not a new config create command

The PR-B spec originally proposed a new config create command. After plan-mode discussion we extended config new instead:

  1. No mental-model fragmentation. A separate config create would have meant two commands with overlapping intent ("make a new config") but different mechanics. The AI agent docs already conflate the two in places.
  2. Reuse of the scaffold pipeline. With --push --output-dir, the same scaffold step that powers scaffold-only mode also produces the files alongside the POST — the GitOps-plus-ID-claim workflow.
  3. Zero regression surface. Without --push, today's behavior is byte-for-byte preserved.
  4. One-flag flip for FIIA. --push --no-files is the FIIA path: no filesystem state, single HTTP POST, returns the new config ID. Direct replacement for the tool call create_config escape hatch, with the bonus that it does not inherit the MCP keboola.snowflake-transformation refusal (the typed CLI wraps the raw Storage API).

The Snowflake-transformation row in keboola-expert.md is also updated: previously the first-choice was config new + config update --set (two-step without remote create in step 1); now config new --push --no-files works in one call for all component types including Snowflake.

Schema validation

  • Validation runs by default when --push is set AND an explicit --configuration / --configuration-file body is given.
  • On mismatch: ConfigError → exit 5, error paths listed.
  • AI Service has no schema, returns an error, or has a malformed schema → silently skip (validation_status: "skipped"); never blocks creation.
  • --no-validate opts out entirely.
  • The default empty-shell case (no body passed, POST {}) auto-skips validation — most component schemas require parameters, so the FIIA empty-shell pattern shouldn't be blocked by predictable failures.
  • --dry-run returns the validation outcome in the envelope but does NOT abort on validation failure (dry-run is inspection-only).

Behavior matrix

flags scaffold to stdout write files API POST
(none, no --push) yes (today) no no
--output-dir D no yes (today) no
--push --project P --name N yes no yes
--push --project P --name N --output-dir D no yes yes
--push --project P --name N --no-files no no yes

Files changed (18)

Serviceservices/config_service.py: new create_config() + _validate_config_body() helper. ConfigService.__init__ overridden to accept an ai_client_factory for testability.

Commandcommands/config.py: extend config_new() with 8 new options + flag-combination validation block. Refactored the disk write + the human-mode push-result renderer into shared helpers.

Hintshints/definitions/config.py: new config.new CommandHint (only emitted when --push is set).

Tests

  • tests/test_config_create_service.py (new) — 15 cases: happy paths, dry-run, schema validation (ok / failed / skipped / no-schema / AI error / malformed schema / validate=False), client cleanup on error.
  • tests/test_config_create_cli.py (new) — 20 cases: flag-combination validation (9), push-mode happy paths (8), error propagation (3).
  • tests/test_e2e.py — new step 19b: dry-run → real create → detail verify → update --set patch → delete cleanup, wrapped in try/finally with pre-registered safety-net teardown.

Sync map (CLAUDE.md convention #17 — all 7 surfaces updated)

  • src/keboola_agent_cli/commands/context.py — inventory + usage note
  • CLAUDE.md ## All CLI Commands — replaced config new inventory line
  • plugins/kbagent/agents/keboola-expert.md — amended Snowflake matrix row + amended inline Snowflake gotcha + new "Create a new config (one-shot remote)" row
  • plugins/kbagent/skills/kbagent/SKILL.md — auto-regenerated via make skill-gen
  • plugins/kbagent/skills/kbagent/references/commands-reference.md — two-mode entry with full flag inventory
  • plugins/kbagent/skills/kbagent/references/gotchas.md — new (since v0.31.1) section
  • plugins/kbagent/skills/kbagent/references/scaffold-workflow.md — dual-mode callout + new push examples in step 3

Version

  • pyproject.toml: 0.31.0 → 0.31.1 (patch — single command extension, no breaking changes)
  • src/keboola_agent_cli/changelog.py: new 0.31.1 entry
  • plugins/kbagent/.claude-plugin/plugin.json, .claude-plugin/marketplace.json: synced via make version-sync
  • uv.lock: refreshed via uv sync

Test plan

  • make check clean — ruff lint, ruff format-check, skill-check, version-check, changelog-check, 2919 tests pass
  • Service tests: 15/15 pass (tests/test_config_create_service.py)
  • CLI tests: 20/20 pass (tests/test_config_create_cli.py)
  • Regression: existing TestConfigNew (5 cases) still passes byte-for-byte
  • E2E against real project 1143 (EU GCP) — step 19b dry-run + real create + detail verify + update patch + delete cleanup all green. Live config IDs created and cleaned up across multiple runs.
  • Manual smoke: --push without --project → exit 2, VALIDATION_ERROR envelope ✓
  • Manual smoke: --push without --name → exit 2 ✓
  • --hint client emits a client.create_config(...) snippet; --hint service emits a service.create_config(...) snippet ✓
  • Two independent review iterations (code + security) — security verdict CONVERGED with 0 material findings; code review had 5 NON-BLOCKING items, all addressed
  • CI green on this PR

Note on E2E pre-existing test failure

The full tests/test_e2e.py::TestFullE2E::test_full_cli_e2e flow currently fails at step 37 (_test_sharing_and_lineage) because the test invokes kbagent lineage show --project P without a required --load argument. This failure is pre-existing — committed on 2026-04-14 by Petr Simecek (line 2499), unrelated to this PR. Step 19b (this PR's new code) passes cleanly in every run; the test harness reaches step 37 only because it's later in the same flow.

🤖 Generated with Claude Code

Extends `kbagent config new` with a `--push` flag that POSTs to the
Storage API in a single CLI call. Without `--push`, today's scaffold-
only behavior is byte-for-byte preserved. Closes the F2 gap in
`kbagent-feature-gaps.md` -- previously the only "FIIA HTTP call ->
kbagent CLI" mapping that lacked a first-class command.

Service: ConfigService.create_config + _validate_config_body helper
  (jsonschema.Draft7Validator against AI Service `configurationSchema`).
  Fail-closed on explicit-body mismatch, graceful skip on missing /
  malformed schema, auto-skip on empty-shell (FIIA pattern).
Command: extend config_new with 8 new flags -- --push, --no-files,
  --description, --configuration JSON|@file|-, --configuration-file PATH,
  --no-validate, --branch ID, --dry-run.
Hint: config.new hint emits create_config snippets only when --push set.
Tests: 35 new (15 service + 20 CLI) + 1 new E2E hop with try/finally
  cleanup; pre-existing TestConfigNew preserved byte-for-byte. Live E2E
  passed against project 1143: dry-run + real create + detail verify +
  update --set patch + delete cleanup.
Docs: 7 sync-map surfaces synced (convention keboola#17); new gotchas entry
  clarifies scaffold-vs-push split + MCP `create_config` Snowflake
  refusal nuance + validation behavior. Snowflake row in
  keboola-expert.md updated to surface --push --no-files as a
  first-choice one-shot path.

Version bump 0.31.0 -> 0.31.1 (patch). plugin.json + marketplace.json
synced via `make version-sync`; uv.lock refreshed via `uv sync`.

@padak padak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review of #278 — feat(0.31.1): config new --push for one-shot remote create

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 CI check status (green on PR),
not duplicated here.

Summary

This PR extends kbagent config new with a --push flag that POSTs to the Storage
API in one call, filling the F2 gap that previously had no first-class command.
The implementation is architecturally clean, test coverage is thorough (35 new unit
tests, comprehensive flag-combination validation, proper E2E try/finally cleanup),
and all 7 silent-drift surfaces from CLAUDE.md convention #17 are updated.

Verdict: REQUEST CHANGES. Two issues must be fixed before merge:
(1) The PR targets the wrong base — main is now at v0.32.0; the PR's v0.31.1 bump
conflicts, and all (since v0.31.1) version tags ship stale after merge.
(2) When --push --output-dir --json is combined, a plain-text dim line is emitted
to stdout before the JSON object, silently breaking JSON consumers on that flag
combination.

Verdict

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

Blocking findings

[B-1] pyproject.toml:4 — Version bump 0.31.1 is obsolete; main is at 0.32.0

The PR increments from 0.31.0 to 0.31.1, but main has merged PR #277 (storage truncate-table, v0.32.0) since this branch diverged. Merging as-is would attempt to
set pyproject.toml version = "0.31.1" against a base that already has
version = "0.32.0", producing a merge conflict (confirmed: mergeStateStatus=DIRTY).
Even if the conflict were manually resolved to keep 0.31.1, it would regress the
already-shipped 0.32.0 entry in changelog.py, plugin.json, and uv.lock.

All 19 occurrences of 0.31.1 in the diff — including gotchas.md's
(since v0.31.1) heading, the matrix entries in keboola-expert.md, and
the scaffold-workflow.md callout — must be re-tagged to the final version
chosen after rebase (0.32.1 for a patch or 0.33.0 for a minor, per the
maintainer's preference). The changelog.py entry must be added under the
new version key, not under "0.31.1".

Fix: rebase onto current main, resolve conflicts, bump pyproject.toml to
0.32.1 (or 0.33.0), run make version-sync, and do a global find-and-replace
of 0.31.1 → chosen version across all doc files.

[B-2] src/keboola_agent_cli/commands/config.py:512--push --output-dir --json emits plain text before JSON, breaking stdout parsers

In the --push path when --output-dir is also set, the command calls:

_write_scaffold_to_disk(formatter, scaffold, output_dir, json_mode=False)

json_mode=False is hardcoded regardless of the caller's --json flag.
_write_scaffold_to_disk with json_mode=False calls
formatter.console.print(f"[dim]Scaffold written to ..."), which writes to
formatter.console — the stdout-bound Console instance (not err_console).
formatter.output(push_result) then writes the JSON object to sys.stdout.

Result on stdout with kbagent --json config new ... --push --output-dir ./scratch:

Scaffold written to ./scratch/extractor/.../test-config (1 file(s))
{
  "status": "ok",
  ...
}

The JSON parser sees the first line as non-JSON and fails. The test
test_push_with_output_dir_scaffolds_and_posts (diff line 1269) uses --json
but only asserts result.exit_code == 0 and file presence — it does not call
json.loads(result.output), so the test passes while the bug ships.

Fix: change the call at line 512 to
_write_scaffold_to_disk(formatter, scaffold, output_dir, json_mode=formatter.json_mode),
matching the scaffold-only path at line 530. Also add
json.loads(result.output) assertions to test_push_with_output_dir_scaffolds_and_posts.

Non-blocking findings

[NB-1] plugins/kbagent/agents/keboola-expert.md:80config new --push missing from VERSION GATE enumeration

The VERSION GATE block (§1 Rule 6) lists minimum-version requirements line by
line so the AI agent can refuse tasks on old installs. After the rebase the block
ends with project edit --new-alias ... needs 0.31.0+ and storage retype is a future composite. The new config new --push feature (version X.Y.Z after rebase)
is not listed there, even though the PR correctly adds 0.31.1+ annotations in the
Tool Selection Matrix rows and in gotchas.md. An AI agent on a pre-X.Y.Z install
will attempt config new --push based on the matrix entry but not be pre-warned by
the gate check.

CONTRIBUTING.md §Releasing a new version step 5 classifies this as informational
(NON-BLOCKING), but the gap should be closed before the PR ships.

Fix: add one line to the VERSION GATE enumeration after needs 0.31.0+:
\config new --push` (one-shot remote create) needs X.Y.Z+,`
(substituting the final version after rebase).

[NB-2] tests/test_config_create_cli.py:1269 + tests/test_config_create_service.py — No test exercises component_id="keboola.snowflake-transformation" specifically

The PR description and agent docs prominently claim that --push bypasses the MCP
create_config refusal for keboola.snowflake-transformation. This claim is
architecturally correct (the service calls client.create_config directly), but
every service-level test uses keboola.ex-db-snowflake (a different component type).
No unit or CLI test passes component_id="keboola.snowflake-transformation" and
asserts the API POST proceeds. The E2E test (step 19b) uses keboola.ex-http.

If the AI Service schema lookup for keboola.snowflake-transformation behaves
differently from keboola.ex-db-snowflake (e.g. returns a refusal-shaped
error rather than KeboolaApiError), the claim would be unverified at unit-test
level and only caught in production. Adding one parametrized test case with
keboola.snowflake-transformation and configuration=None (the empty-shell path)
would pin the documented behavior.

[NB-3] src/keboola_agent_cli/services/config_service.py:929except Exception: in validation fallback is overly broad

_validate_config_body contains:

except Exception:
    logger.warning(...)
    return ("skipped", [])

The comment correctly explains why (late-raising UnknownType from iter_errors),
but a bare except Exception will also silently swallow programming errors such as
AttributeError, TypeError, or MemoryError during the validation loop,
turning them into silent skips. The intent is to catch validation-runtime errors
from jsonschema's lazy evaluation. Narrowing to
except (jsonschema.ValidationError, jsonschema.SchemaError, Exception) is not
better, but catching jsonschema.exceptions.SchemaError plus
jsonschema.exceptions.ValidationError plus an explicit except Exception with a
comment tying it specifically to the known UnknownType failure mode would make
the intent auditable.

Per the severity rubric this is NON-BLOCKING (the intent is documented in the
comment; the behavior is fail-safe rather than fail-dangerous).

Nits

  • [NIT-1] src/keboola_agent_cli/commands/config.py:527assert scaffold is not None in production code. The comment explains this is a type-narrowing assist, which is fine, but assert statements are suppressed with python -O. Using if scaffold is None: raise RuntimeError(...) would make the guard unconditional without changing the semantics.

  • [NIT-2] PR description ends with 🤖 Generated with [Claude Code](https://claude.com/claude-code). Per CONTRIBUTING.md > "Commit & PR Conventions": "No AI attribution footers in PR descriptions." This should be removed before merge.

Verification log

  • gh auth status → padak account active, repo scope confirmed ✓
  • gh pr view 278 --repo padak/keboola_agent_cli --json state,mergeable,mergeStateStatusstate=OPEN, mergeable=CONFLICTING, mergeStateStatus=DIRTY ✓ (merge conflict confirmed)
  • gh pr diff 278 --repo padak/keboola_agent_cli > /tmp/pr278.diff → 2099 lines, 18 files, +1772/-62 ✓
  • git log --oneline -3 on origin/main → 8aeeb10 feat(0.32.0): storage truncate-table; pyproject.toml on main = version = "0.32.0" ✓ (B-1 version collision confirmed)
  • PR diff pyproject.tomlversion = "0.31.1"
  • 3-layer check: grep typer/click/formatter/console.print in config_service.py additions → zero hits ✓ (no layer violation in service)
  • 3-layer check: grep httpx in commands/config.py additions → zero hits ✓
  • grep "config\.new" /tmp/pr278.diffhints/definitions/config.py registers CommandHint with cli_command="config.new"
  • gh api repos/ottomansky/keboola_agent_cli/contents/src%2Fkeboola_agent_cli%2Fpermissions.py?ref=feat%2Fconfig-create"config.new": "write" at line 42 ✓ (permission entry present)
  • VERSION GATE block on fork branch (lines 62-85) → config new --push NOT listed ✓ (NB-1 confirmed)
  • commands/config.py diff line 512 → _write_scaffold_to_disk(..., json_mode=False) hardcoded ✓ (B-2 confirmed)
  • output.pyself.console = Console(...) (stdout, not stderr) ✓ (B-2 stdout pollution confirmed)
  • test_push_with_output_dir_scaffolds_and_posts → asserts exit_code == 0 and written.exists() only; no json.loads(result.output) ✓ (test gap for B-2 confirmed)
  • grep "keboola\.snowflake-transformation" in test files → 0 hits in test assertions (only in docs/prose) ✓ (NB-2 confirmed)
  • except Exception: in config_service.py:929 → confirmed, correctly guarded with logger.warning ✓ (NB-3 noted)
  • TEST_TOKEN = "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k" → same synthetic token used across 10+ existing test files (e.g. test_workspace_cli.py:22, test_errors.py:11); not a real credential ✓
  • gh pr checks 278 --repo padak/keboola_agent_clicheck pass 1m44s ✓ (CI green on the PR's own branch; make check on working tree fails only due to unrelated 0.32.0 changelog gap on the local branch)
  • E2E step 19b try/finally: new_config_id captured before assertions (diff line 2032); registered in _created_config_ids before try: block; finally: runs config delete ✓ (teardown ordering correct)
  • Snowflake bypass claim: client.create_config exists on main at client.py:691; service calls it without any component-type filter ✓ (claim is architecturally correct; unit test coverage gap NB-2 noted)
  • Behavior verification: not run against real API due to cross-fork PR and missing live credentials; E2E author-confirmed against project 1143 per PR description.

Open questions for the author

(none)

@padak

padak commented May 12, 2026

Copy link
Copy Markdown
Member

Hi @ottomansky — heads up: this PR is being consolidated with #279 into a single 0.33.0 joint release at #282.

What happened:

The merge-conflict resolution kept BOTH new VERSION GATE entries in keboola-expert.md (your data-app * line from #279 sits next to the storage truncate-table line from #277, plus a new config new --push line for this PR's contribution).

Plan: once #282 merges, this PR will be closed as superseded. Your Git authorship is preserved on both feature commits in #282; the release notes for 0.33.0 explicitly credit you for the implementation, live-API validation, and the 7-surface doc sync.

Thanks for the work on this — --push is a great FIIA migration unblock, especially the Snowflake-transformation matrix fix.

@padak

padak commented May 12, 2026

Copy link
Copy Markdown
Member

Superseded by #282 (joint 0.33.0 release). See comment above for context.

@padak padak closed this May 12, 2026
padak added a commit that referenced this pull request May 12, 2026
…se of #278 + #279) (#282)

* feat(0.31.1): config new --push for one-shot remote create

Extends `kbagent config new` with a `--push` flag that POSTs to the
Storage API in a single CLI call. Without `--push`, today's scaffold-
only behavior is byte-for-byte preserved. Closes the F2 gap in
`kbagent-feature-gaps.md` -- previously the only "FIIA HTTP call ->
kbagent CLI" mapping that lacked a first-class command.

Service: ConfigService.create_config + _validate_config_body helper
  (jsonschema.Draft7Validator against AI Service `configurationSchema`).
  Fail-closed on explicit-body mismatch, graceful skip on missing /
  malformed schema, auto-skip on empty-shell (FIIA pattern).
Command: extend config_new with 8 new flags -- --push, --no-files,
  --description, --configuration JSON|@file|-, --configuration-file PATH,
  --no-validate, --branch ID, --dry-run.
Hint: config.new hint emits create_config snippets only when --push set.
Tests: 35 new (15 service + 20 CLI) + 1 new E2E hop with try/finally
  cleanup; pre-existing TestConfigNew preserved byte-for-byte. Live E2E
  passed against project 1143: dry-run + real create + detail verify +
  update --set patch + delete cleanup.
Docs: 7 sync-map surfaces synced (convention #17); new gotchas entry
  clarifies scaffold-vs-push split + MCP `create_config` Snowflake
  refusal nuance + validation behavior. Snowflake row in
  keboola-expert.md updated to surface --push --no-files as a
  first-choice one-shot path.

Version bump 0.31.0 -> 0.31.1 (patch). plugin.json + marketplace.json
synced via `make version-sync`; uv.lock refreshed via `uv sync`.

* fix(0.31.1): data-app --json emits app_id (renamed from id) for --app-id input/output symmetry

BREAKING (output-only). Every `kbagent --json data-app <subcommand>` envelope
now emits the data-app's own identifier under the key `app_id`. Pre-0.31.1 the
same key was named bare `id`, which did not match the `--app-id` input flag
and surprised AI agents templating `jq -r '.apps[].app_id'`. Affects 12
subcommands: list, detail, create, deploy, start, stop, delete, password,
secrets-set, secrets-list, secrets-get, secrets-remove. `config_id` is
unchanged. The Storage config back-pointer at `parameters.id` (writeup §5;
lives INSIDE the configuration body sent TO Storage, not in kbagent output)
is unchanged.

Pipe-friendly chain enabled:
  kbagent --json data-app list | jq -r '.apps[].app_id' \
    | xargs -I{} kbagent data-app deploy --project P --app-id {}

Migration: rename `.id` -> `.app_id` in jq filters / scripts. No deprecation
alias is emitted; no in-tree consumers found.

Live-validated 2026-05-12 against europe-west3.gcp projects 1143, 2738, 2959.
The more dramatic null-id symptom in the original v0.30.2 report could NOT be
reproduced -- v0.30.6 is byte-identical to v0.30.2 for these files, and the
Data Science API on europe-west3.gcp serves all-camelCase keys. This release
is scoped to the rename only. Reported by @ottomansky.

Structural changes:
- Service: 9 envelopes in services/data_app_service.py rewrite "id" -> "app_id"
- Command: 4 human-formatter sites in commands/data_app.py follow the rename
- Tests: new TestDataAppListOutputKeys + 11-test TestDataAppEnvelopesNoBareIdKey
  regression class in tests/test_data_app_service.py; CLI + secrets-CLI fixture
  renames; E2E list-step round-trip assertion in test_data_app_lifecycle_public
- Docs: (since v0.31.1) gotcha entry, commands-reference data-app callout,
  keboola-expert VERSION GATE Rule 6 entry, changelog
- Version: pyproject 0.31.0 -> 0.31.1, plugin.json + marketplace.json synced

* chore(0.33.0): bump version + PR #278 review fixes + combined release prep

This commit prepares the joint 0.33.0 release that bundles two cherry-picked
contributions from @ottomansky:
  - PR #278 (feat: config new --push for one-shot remote create)
  - PR #279 (fix: data-app --json emits app_id, renamed from id)

Both PRs originally targeted 0.31.1 against a base before #277 (truncate-table)
landed; rebased onto current main (v0.32.0) and renumbered to 0.33.0 minor.

Changes in this commit (on top of the two cherry-picked author commits):

- Version: 0.32.0 -> 0.33.0 in pyproject.toml, propagated via
  make version-sync to plugin.json + marketplace.json; uv.lock refreshed.
- Doc references: all 12 occurrences of "0.31.1" rewritten to "0.33.0"
  across keboola-expert.md (VERSION GATE Rule 6, Tool Selection Matrix,
  inline Snowflake gotcha), gotchas.md ((since v0.33.0) section headers),
  commands-reference.md, scaffold-workflow.md, commands/context.py.
- keboola-expert.md VERSION GATE Rule 6: kept both new entries
  ("storage truncate-table needs 0.32.0+" from #277; "data-app * JSON
  output uses key app_id on 0.33.0+" from #279; "config new --push needs
  0.33.0+" from #278) -- merge-conflict resolution kept both, not either-or.
- gotchas.md: kept both new (since v0.33.0) sections side by side
  (config new --push scaffold-vs-push split + MCP refusal nuance;
  data-app app_id rename rationale + camelCase wire-key note).
- changelog.py: new 0.33.0 entry with 5 bullets covering both features,
  combined test count (47 new tests: 15 service + 20 CLI for #278, 12
  for #279), 7-surface doc sync, and the review fixes below.

Review fixes (addresses PR #278 review B-2 blocker):

- src/keboola_agent_cli/commands/config.py:1334 -- the --push --output-dir
  path called _write_scaffold_to_disk(..., json_mode=False) unconditionally,
  which emitted a plain-text "Written: ..." dim line above the JSON envelope
  when --json was set, silently breaking jq pipes. Now honors
  formatter.json_mode like every sibling call site.
- tests/test_config_create_cli.py::test_push_with_output_dir_scaffolds_and_posts
  now asserts json.loads(result.output) parses cleanly, locking the
  regression behavior down.

Closes (will be closed when this branch merges): #278, #279.

* fix(0.33.0): silent scaffold-write on --push --output-dir (avoids double-JSON)

The previous review-fix attempt for B-2 changed the caller from
`json_mode=False` to `json_mode=formatter.json_mode`. That swap closed
the plain-text leak in human mode but introduced the opposite regression
in JSON mode: `_write_scaffold_to_disk` then emitted its own
`{directory, files_written}` JSON envelope and the push-result envelope
right after, producing two concatenated JSON documents on stdout and
breaking `jq` consumers.

The function's docstring already pointed at the correct design: in
scaffold+push mode, the caller is supposed to suppress the
scaffold-banner side-channel because the push-result envelope is the
authoritative output. The clean primitive for that is an explicit
`silent=True` parameter -- write files, emit nothing.

- `_write_scaffold_to_disk` gets an optional `silent: bool = False`
  parameter. When True, the file write proceeds and the function
  returns before any banner/envelope emission. Default behavior
  (scaffold-only mode caller at line 1345) is unchanged.
- The `--push --output-dir` caller now passes `silent=True`. The
  push-result envelope (single JSON in --json mode; "Created config ..."
  line in human mode) is the only output for this path.
- The regression test
  `test_push_with_output_dir_scaffolds_and_posts` already asserts
  `json.loads(result.output)` parses cleanly -- now green.

Closes the B-2 finding from the PR #278 review.

* fix(0.33.0): PR #282 review follow-ups (dry-run guard, cleanup, test labels)

Addresses three findings from the kbagent-pr-reviewer pass on PR #282:

- NB-1 (`commands/config.py:1338`): the ``--push --output-dir`` scaffold
  write branch had no ``and not dry_run`` clause, so
  ``--push --output-dir --dry-run`` silently created files on disk
  alongside the preview envelope. Dry-run is a preview; it must have
  zero filesystem side effects. New regression test
  ``test_push_output_dir_dry_run_does_not_write_files`` asserts the
  envelope reports ``dry_run: True`` AND no ``*.yml`` / ``*.json`` files
  exist under output_dir after the invocation.
- NIT-1 (same call site, line 1340): ``json_mode=formatter.json_mode``
  was dead code -- when ``silent=True`` the helper returns before
  reading ``json_mode``. Replaced with a sentinel ``json_mode=False`` and
  a comment explaining why the argument is required by the signature
  but ignored by the body.
- NB-2: 9 leftover ``0.31.1`` references in ``tests/test_data_app_service.py``
  (lines 920, 925, 927, 960, 968 -- class comment block and the
  ``TestDataAppListOutputKeys`` docstring) and ``tests/test_e2e.py``
  (lines 440, 1659, 1661, 6525 -- ``_step`` label, method docstring,
  inline comment). Renumbered to ``0.33.0``; the rewrite scope in
  commit 18dbcbb was ``src/`` + ``plugins/`` only, ``tests/`` was
  intentionally out-of-scope at the time, this commit closes that gap.

Deferred (acknowledged but not in this commit):
- NB-3 (no ``keboola.snowflake-transformation`` unit test for ``--push``):
  the claim is correct by design (the service calls
  ``client.create_config`` directly with no Snowflake-specific branch);
  an explicit regression test is nice-to-have but not blocking. Tracked
  as a follow-up.
- NIT-2 (changelog bullet 5 wording on the ``silent=True`` mechanism):
  the bullet is technically correct but undersells the structural fix.
  Cosmetic; not worth re-publishing the changelog for.

``make check`` clean -- 2953 tests pass (one more than before, the
new dry-run guard test).

---------

Co-authored-by: ottomansky <ottomansky.max@gmail.com>
@ottomansky
ottomansky deleted the feat/config-create branch May 12, 2026 15:38
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.

2 participants