feat(config): add state-get/state-set, guard --set against API siblings

[padak]

Closes #593.

What this solves

kbagent could read a configuration's runtime state (config detail --with-state) but had no way to write it, so seeding or resetting state was only reachable by clicking in the KBC UI (/raw → Update State). And the closest-looking CLI path silently wrote to the wrong place.

Part A — --set no longer silently no-ops

config update --set 'state.x=y' used to exit 0, bump the config version and show a plausible --dry-run diff, while writing to configuration.state.x and leaving the runtime state untouched — a write that looked successful at every observable layer while doing nothing.

Now any --set path whose first segment is a top-level sibling of configuration (state, rows, name, description, id, version, currentVersion, changeDescription, created, creatorToken, isDeleted, isDisabled) is rejected with exit 2, before any network call, so --dry-run is rejected too. The error names the offending path and routes to the right tool (state.* → config state-set, name → --name, …), and mentions --configuration JSON|@file|- as the escape hatch if a component genuinely has such a key inside configuration.

set_nested_value also stops silently turning files[0] into a literal "files[0]" key and points at the supported files.0 form.

Note: the guard also covers config row-update --set, which the issue did not mention — it accepts --set too, so the footgun was wider than reported.

Part B — config state-get / config state-set

kbagent config state-get --project P --component-id C --config-id ID [--row-id R] [--branch ID]
kbagent config state-set --project P --component-id C --config-id ID [--row-id R] \
    --state JSON|@file|- [--branch ID] [--dry-run] [--yes]

Calls the dedicated branch-scoped Storage API state endpoints, with --row-id routing to the row endpoint. The payload is validated as a JSON object under 4 MB before the round-trip, --dry-run previews a current-vs-new diff (same shape as config update --dry-run), an unchanged state short-circuits with changed: false and no API call, and a missing row id fails loudly instead of returning {}.

Confirmation follows the established config row-delete pattern: prompts interactively, --json skips without requiring --yes, --dry-run never prompts.

A bug the live E2E caught that mocks could not

state-set --row-id initially reported a false NOT_FOUND even though the PUT returned 200 and the state landed on the server.

The two endpoints answer with different envelopes: the root PUT returns the full configuration detail (which carries a rows[] array), the row PUT returns the bare row object. The service looked the row back up inside result["rows"] in both cases, so every row write missed.

Two unit tests actually confirmed the bug, because they mocked a full config detail for the row endpoint — they asserted the author's assumption about the API rather than the API. They started failing once the bug was fixed, which was the right signal. Both response shapes are now pinned by regression tests that deliberately mock the real shapes.

Worth noting the symmetry with the issue itself: #593 describes a write that reported success and did nothing; this one did the work and reported failure. Same root cause — code trusting a response shape it never verified.

Motivation

Keboola is retiring processed_tags/query in file input mapping in favour of changed_since: adaptive. adaptive with an empty state reloads the entire file history, and a dev branch always starts with state: {} — so validating that migration safely requires seeding a known lastImportId first. That was the only manual step in an otherwise scriptable, deadline-driven migration every affected customer has to perform.

Testing

• Live E2E against project 5946, all 10 steps: fresh {} → dry-run (no write) → write → read-back → no-op → row write/read + root/row independence → missing row errors → guard exit 2 + regression → cleanup. Script added at ~/kbagent/e2e/config_state_e2e.py (not in repo).
• TestE2EConfigState in tests/test_e2e.py per coding convention Close must-have gaps in explorer command #16.
• 43 unit tests (client / service / CLI) + 57 guard tests.
• make check green: 5662 passed, 12 skipped.

Verified against the live apiary spec (keboola/storage-api-php-client), not just the issue: the state endpoints take a genuine JSON body ({"state": {...}}), not the form-encoded json.dumps shape update_config uses for configuration — copying that pattern would have broken the call.

Scope deviations, flagged deliberately

• commands/config_state.py is a new file. commands/config.py is CI-ratcheted to shrink only; adding the guard alone broke the ratchet. Rather than bump the baseline (which the tooling explicitly forbids for a file you just grew), the new commands live in their own module and 13 duplicated try/except blocks in config.py were folded into a shared error-mapping helper. The diff is symmetric — 13 identical pairs replaced, same exit-code mapping — and config.py went 2007 → ~1938 code lines.
• services/config_service.py is at 1497/1500 code lines. Under the hard ceiling, but the next PR adding material here should split it first.
• plugins/kbagent/agents/keboola-expert.md untouched — it sits at 61985 of its 62000-byte budget, and per CONTRIBUTING.md adding a command to an existing group needs no new matrix row. That file is effectively at its ceiling and deserves a separate trim.

Version

Per owner instruction: no version bump in this PR. The changelog entry is filed under 0.84.2; the bump happens jointly after this and #594 are merged.

Minor, non-blocking

The 4 MB size check uses >=, so a state of exactly 4 MiB is rejected where the API spec ("maximum allowed size is 4MB") might still accept it — one byte stricter, in the safe direction.

---

[devin-ai-integration]

Devin Review found 3 potential issues.

[padak]

Review of #598 — feat(config): add state-get/state-set, guard --set against API siblings

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 adds config state-get/config state-set (dedicated Storage API PUT .../state endpoint) and closes a real footgun where config update --set 'state.x=y' silently wrote to configuration.state.x and left runtime state untouched (exit 0, plausible dry-run diff, nothing actually changed). It also rejects the files[0] bracket-syntax typo that previously created a bogus literal key. Documentation sync (CLAUDE.md, context.py, SKILL.md, commands-reference.md, gotchas.md, a new config-state-workflow.md), permissions registry, and server router are all updated — this is one of the most thorough silent-drift-conscious PRs I've reviewed. Verdict: REQUEST CHANGES — one BLOCKING finding: the new bracket-syntax rejection in set_nested_value raises a bare ValueError that is not caught by either config update's or config row-update's exception handler, so it crashes with an unhandled traceback (exit 1, no JSON output at all under --json) instead of the clean usage-error the rest of the guard work in this same PR is designed to produce. Everything else — the sibling-path guard placement/coverage, the state read/row-envelope fix, the confirmation-prompt semantics, and the config.py error-handling refactor — checked out correctly.

Verdict

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

Blocking findings

[B-1] src/keboola_agent_cli/json_utils.py:70 — bracket-syntax ValueError is uncaught, crashes config update --set/config row-update --set with a raw traceback under --json

set_nested_value now raises a bare ValueError for any --set path containing [/] (e.g. --set 'files[0]=z'). This function is called from ConfigService._resolve_configuration (services/config_service.py:933) and _resolve_row_configuration (services/config_service.py:2329), both invoked from commands/config.py's config_update (line 799-800) and config_row_update (line 2367-2368), whose except clauses only catch (ConfigError, KeboolaApiError) — not ValueError. I reproduced this live via CliRunner with a mocked client: kbagent --json config update --project P --component-id C --config-id ID --set 'files[0]=z' (past the sibling-path guard, since files isn't a guarded prefix, and past client.get_config_detail so it's fully reachable) exits with code 1 and empty stdout — no JSON error envelope at all, violating the "Dual output... never print raw text that breaks JSON parsing" convention (CONTRIBUTING.md) and the exit-code contract (2=usage error is what this exact family of validation errors should produce, matching the sibling-path guard's own typer.Exit(code=2)). This is new code introduced by this PR (previously bracket syntax silently created a bogus key — wrong, but not a crash), so it's a regression in kind, not a pre-existing issue. Also confirmed: only unit-level tests exist for the ValueError (tests/test_config_set_guard.py::TestSetNestedValueBracketGuard) — none exercise it through the CLI layer, which is exactly why this gap wasn't caught.

Fix: catch ValueError in config_update/config_row_update (or centralize in _guard_set_paths/a new helper) and map it to formatter.error(error_code=ErrorCode.VALIDATION_ERROR, ...) + typer.Exit(code=2), consistent with the sibling-path guard's own exit code. Add a CLI-layer regression test (CliRunner invocation, --json mode, assert structured JSON error + exit 2) alongside the existing unit tests.

Non-blocking findings

(none)

Nits

• [NIT-1] services/config_service.py:1221 — the >= bound on the 4 MB state-size check (if size >= CONFIG_STATE_MAX_BYTES) rejects a state of exactly 4 MiB, one byte stricter than the API spec's "maximum allowed size is 4MB." Already flagged by the author in the PR description as intentional/safe-direction; verified the code matches that description exactly. No action needed.

Verification log

• gh pr view 598 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → OPEN, feat(config): prefix matches new-behavior scope, +3266 lines across 18 files ✓
• Read CONTRIBUTING.md (Checklist: Adding a New CLI Command, Plugin synchronization map, Releasing a new version) and CLAUDE.md convention #17 + ## All CLI Commands ✓
• Read plugins/kbagent/agents/keboola-expert.md §1 (non-negotiable rules) and §2/§3 — no new write/destructive command group introduced (single command inside existing config area), so per CONTRIBUTING.md a missing matrix row is expected/non-issue; author explicitly documented the 61985/62000-byte budget constraint in the PR description ✓
• git rev-parse --abbrev-ref HEAD → claude/issue-593-analysis-725047, matches <branch> ✓ (no gh pr checkout needed)
• gh pr diff 598 → 3725-line diff, 18 files touched (CLAUDE.md, SKILL.md, commands-reference.md, new config-state-workflow.md, gotchas.md, changelog.py, client/configs.py, commands/config.py, new commands/config_state.py, commands/context.py, constants.py, json_utils.py, permissions.py, server/routers/configs.py, services/config_service.py, 4 test files) ✓
• 3-layer compliance grep (typer/click/formatter in services/, httpx/requests in commands/, formatter/typer in clients) → all empty, no violations ✓
• Plugin synchronization map walk: commands/context.py AGENT_CONTEXT ✓ updated with full state-get/state-set docs; CLAUDE.md ## All CLI Commands ✓ updated with since-tag; permissions.py OPERATION_REGISTRY ✓ has config.state-get: read / config.state-set: write; server/routers/configs.py ✓ has matching GET/PUT /state routes (1:1 mirror); commands-reference.md ✓ updated; gotchas.md ✓ two new entries both tagged (since v0.84.2); new config-state-workflow.md (208 lines) created for the new topic ✓
• Reproduced the --set 'state.x=y' sibling-path guard: traced validate_set_paths call sites — both _resolve_configuration (line 919) and _resolve_row_configuration (line 2313) call it as their first line, before any client.get_config_detail call, so --dry-run cannot bypass it; CLI layer (_guard_set_paths) also calls it before service dispatch as defense-in-depth. Guarded-prefix list (state, rows, name, description, id, version, currentVersion, changeDescription, created, creatorToken, isDeleted, isDisabled) matches the real Storage API config-detail top-level fields; only the first dot segment is checked, so parameters.state.x (a legitimate component parameter) is unaffected ✓
• Reproduced the root-vs-row response-envelope fix: _extract_state(result, None, component_id, config_id) at services/config_service.py:1271 correctly reads state from result's top level for BOTH the root-PUT full-detail envelope and the row-PUT bare-row envelope, since both carry state at their own top level — confirmed correct, matches PR description and is pinned by tests/test_config_state.py::test_row_write_returns_state_from_bare_row_response ✓
• Reproduced the _handle_config_service_error refactor: diffed all 13 replaced try/except blocks in commands/config.py — each old block was except ConfigError: ... exit(5) + except KeboolaApiError: ... map_error_to_exit_code(exc), byte-for-byte preserved in the shared helper; no exit-code or exception-scope drift ✓
• Reproduced [B-1] live: wrote a throwaway pytest using CliRunner + a mocked KeboolaClient (patched at keboola_agent_cli.services.base.KeboolaClient) and invoked kbagent --json config update --project prod --component-id c --config-id id --set 'files[0]=z' → exit_code=1, stdout="" (empty), result.exception=ValueError(...) — confirmed the crash is real and not just a hypothetical read of the code ✓
• Confirmation-prompt semantics: commands/config_state.py:124 — if not dry_run and not yes and not formatter.json_mode: — confirmed --json skips the prompt without requiring --yes, --dry-run never prompts (checked first), matching config row-delete's established pattern (which lacks the dry_run clause only because that command has no --dry-run flag at all) ✓
• 4 MB >= bound: confirmed at services/config_service.py:1221, matches the PR's own "Minor, non-blocking" disclosure exactly ✓
• make loc-report → commands/config.py 1939 code lines (down from 2007, baselined-hard-ceiling file correctly shrinking per the ratchet); services/config_service.py 1497/1500 (under hard ceiling, soft-ceiling warning only, as author disclosed); commands/config_state.py 134 lines (new, well under budget) ✓
• uv sync --extra server then make check → 5662 passed, 12 skipped, 153 deselected, 18 warnings in 114.98s, exit 0 — matches the PR description's reported numbers exactly ✓ (includes lint, format-check, typecheck, skill-check, version-check, command-sync-check, changelog-check, check-error-codes, check-sentinel-guards, loc-check, full test suite)
• E2E coverage: confirmed TestE2EConfigState in tests/test_e2e.py (4 test methods: root roundtrip, row roundtrip, missing-row error, guard exit-2 regression) per coding convention #16 — could not run live (no E2E_API_TOKEN/E2E_URL in this sandbox); PR description reports it was run live against project 5946, 10-step script, and this was in fact how the row-envelope bug was originally caught (mocks had confirmed the wrong assumption) ✓ trusted per description, not independently re-run
• permissions.py OPERATION_REGISTRY: config.state-get: read, config.state-set: write present ✓ (write, not destructive — correct, this overwrites a mutable runtime field, not a destroy operation)

Open questions for the author

(none)

[padak]

Thanks — the blocking finding was real and is fixed in 6b6fbf2. Reproduced it first (it only triggers on an existing config, since set_nested_value runs after the fetch):

$ kbagent --json config update ... --set 'files[0]=z' --dry-run
ValueError: Invalid path 'files[0]': bracket syntax like 'files[0]' is not supported...
EXIT: 1     # Rich traceback, no structured JSON

You put your finger on the irony: a PR about turning a silent failure into a loud one had introduced a new unstructured one.

Fix — in the guard, not in a try/except at the command layer. Bracket syntax is the same class of usage mistake as a sibling path, so it now fails in the same place (validate_set_paths), with the same exit code, before the same boundary (any network call), and with the same structured error. Catching it in commands/config.py would have meant two different paths for two identical kinds of error, plus a wasted API round-trip before the rejection.

$ kbagent --json config update ... --set 'files[0]=z' --dry-run
{"status": "error", "error": {"code": "INVALID_ARGUMENT",
 "message": "--set 'files[0]' uses bracket syntax, which is not supported. Use dot-separated integer segments over an existing list instead, e.g. 'files.0' rather than 'files[0]'.", ...}}
EXIT: 2

The ValueError stays in set_nested_value as defence in depth for other callers (e.g. the SDK).

Four regression tests added: structured JSON + exit 2, --dry-run also rejected, row-update covered too, and files.0 still works.

Also acted on the file-size nit properly. The fix pushed config_service.py to 1508/1500, and rather than shave a line I split validate_set_paths into services/_config_set_guard.py — which is what CONTRIBUTING asks for ("the next PR adding material here should split it first"). It is a self-contained, framework-free unit with no dependency on ConfigService state, and config_service.py re-exports the symbol so existing importers are unaffected. check_file_size.py now reports no FAIL.

Re-verified after the move: make check green (5666 passed, 12 skipped) and the live E2E against project 5946 still passes all 10 steps.

On the two points you cleared — the root-vs-row envelope fix and the _handle_config_service_error refactor — the envelope asymmetry was itself found by that live E2E run, after two unit tests had confirmed the bug by mocking the shape the author assumed rather than the shape the API sends. Those mocks are now pinned to the real shapes.