fix(storage): reject malformed describe-batch --from-file shapes with a structured error (#640)

[padak]

What

storage describe-batch --from-file now validates the parsed YAML document's
shape before the first write and rejects a malformed one with a structured
error (INVALID_ARGUMENT, exit 2) instead of crashing.

Why

A section that was a list instead of a mapping reached the write loop and died
on .items():

# bad.yaml -- tables is a list of objects
tables:
  - table_id: in.c-test.part_verify_0880
    columns:
      id: Surrogate key

kbagent --json storage describe-batch --project P --from-file ./bad.yaml
# AttributeError: 'list' object has no attribute 'items'
#   storage_service.py in describe_batch -> for table_id, desc in tables.items()

Two problems, both about the failure mode rather than the rejection:

1. --json was answered with a Rich traceback. A programmatic consumer
   (the serve REST layer, a scheduled agent task, a CI script) got an
   unparseable blob where every other command in this repo emits an envelope.
2. The traceback named tables.items() deep inside the service, not the key
   of the file that was wrong -- the one thing the author needed.

Same hole existed for buckets:, columns:, a document that is not a mapping
at all, and a container where a description string belongs (which used to be
str()-ed into garbage and written to the API).

New behavior

Same input now:

$ kbagent --json storage describe-batch --project P --from-file ./bad.yaml
{"status": "error", "error": {"code": "INVALID_ARGUMENT", "message":
 "'tables' must be a mapping of table ID to description, got a list. Expected:\ntables:\n  in.c-sales.orders: All sales orders"}}
$ echo $?
2

• Validation lives in a new services/_describe_batch_input.py
  (storage_service.py is over its file-size budget, so the pure
  input-handling is what moves out -- same precedent as _storage_tables.py).
  It returns a frozen DescribeBatchInput dataclass with descriptions already
  coerced to str, so the write loop stays type-free.
• It raises ValueError, which is exactly what the command already maps to
  ErrorCode.INVALID_ARGUMENT + exit 2 (the missing-file path) -- no new
  wiring, no new error code.
• Messages name the offending key (columns.in.c-sales.orders.order_id), the
  actual type in YAML vocabulary (got a list / a string / a mapping), and
  a copy-pasteable example of the right shape.
• A non-mapping columns entry used to be a soft per-item error, so a bad file
  could half-apply. It is now a fail-fast usage error like the rest: nothing is
  written, so fixing the file and re-running is always safe.
• Unchanged: API failures stay partial-failure-tolerant (errors[],
  exit 1). Only shape errors abort.

No version bump and no changelog.py change (0.89.0 is assembled separately).
Doc surfaces that stated the tolerance without qualifying it -- gotchas.md,
storage-describe-workflow.md, commands-reference.md -- were corrected in
place; no new doc surface was added.

Tests

tests/test_storage_describe_service.py -- new TestDescribeBatchShapeValidation
(each asserts the message and that no API write was attempted):

• test_tables_as_list -- the exact repro from the issue
• test_buckets_as_list
• test_columns_as_list
• test_columns_entry_scalar / test_columns_entry_list
• test_column_description_is_a_container
• test_table_description_is_a_container
• test_top_level_scalar
• test_malformed_yaml_syntax
• test_nothing_is_applied_when_a_later_section_is_malformed -- a bad
  columns: must not let an earlier valid buckets: through
• test_valid_file_still_applies -- happy-path regression, including the
  non-string scalar (2026) still coercing to "2026"

tests/test_storage_describe_cli.py -- test_describe_batch_malformed_shape_json_envelope:
end-to-end through the CLI with the real StorageService and a client
factory that raises if reached, asserting exit 2, status: error,
code: INVALID_ARGUMENT, and the key/type in the message.

make check passes: lint, format, typecheck, skill/version/command-sync/
changelog/error-code/sentinel/file-size gates, and 5874 tests.

Closes #640

---

[devin-ai-integration]

Devin Review found 1 potential issue.

[padak]

Review of #645 — fix(storage): reject malformed describe-batch --from-file shapes with a structured error (#640)

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 moves storage describe-batch --from-file shape validation up front (new services/_describe_batch_input.py, a frozen DescribeBatchInput dataclass) so a malformed YAML document (tables:/buckets:/columns: as a list, a non-mapping columns entry, a description that is itself a container, or a non-mapping top-level document) is rejected whole with INVALID_ARGUMENT / exit 2 instead of crashing on .items() with a Rich traceback under --json. It deliberately changes a previously-tolerant path (a bad columns: entry used to be a soft per-item error that let the rest of the file half-apply) into a fail-fast usage error, and updates gotchas.md, storage-describe-workflow.md, and commands-reference.md to state that the existing partial-failure tolerance now covers API failures only. Verdict: COMMENT. The implementation, tests, and doc updates are solid and internally consistent; the only gap found is the --help docstring on the Typer command itself, which still describes the old blanket tolerance.

Verdict

• Verdict: COMMENT
• Blocking findings: 0
• Non-blocking findings: 1
• Nits: 0

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/commands/_storage_describe.py:302 — --help docstring still claims blanket per-item tolerance

The Typer command docstring for storage describe-batch still reads "All sections are optional. A failure in one item does not abort the rest -- all results are collected and reported." Verified live (kbagent storage describe-batch --help) -- this text is unqualified and now contradicts the new fail-fast-on-malformed-shape behavior that gotchas.md, storage-describe-workflow.md, commands-reference.md, and the storage_service.py docstring were all correctly updated to describe in this same PR. --help is the "ultimate fallback when documentation drifts" per CONTRIBUTING.md's CLI-surface checklist, and it is now the one surface out of sync. Suggest appending a sentence like the one added to storage_service.py: "This tolerance covers per-item API failures only -- a malformed file section is rejected whole before any write."

Nits

(none)

Verification log

• gh pr view 645 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state -> OPEN, base main, head fix/issue-640-describe-batch-validation, 7 files, +402/-34, conventional fix(storage): matches a bug-fix change ✓
• Diff scope cross-checked against PR description (a) malformed-shape validation, (b) _describe_batch_input.py extraction, (c) 3 doc-surface corrections, (d) tests -- diff contains exactly these and nothing extraneous ✓
• git worktree add --detach at FETCH_HEAD of origin/fix/issue-640-describe-batch-validation (491734d) in an isolated scratch dir -- invoking session's own worktree HEAD never moved ✓
• Layer-violation greps (typer/click/formatter in services/, httpx/requests in commands/, formatter/typer in HTTP clients) -> all empty, no violations ✓
• Convention greps (magic numbers, raw error_code="..." strings, bare except:, print() in prod code, token leakage, new heterogeneous tuple[...] returns) -> all empty; the two -> tuple[...] annotations that did appear are in test-helper functions (_service/_run in the new TestDescribeBatchShapeValidation class), matching an existing convention used in 33 other test files -- not flagged ✓
• make version-sync / uv sync --extra server in the isolated worktree, then make check -> exit 0: ruff check clean, ruff format clean (450 files), ty check 1 pre-existing warning (hatchling unresolved-import in scripts/hatch_build.py, unrelated to this diff, warning-only per CONTRIBUTING), SKILL.md up to date, version in sync, 419/419 version gates resolve, check_command_sync.py: all 262 CLI commands registered + documented (confirms no new/renamed command surface introduced by this PR -- OPERATION_REGISTRY, CLAUDE.md, context.py, commands-reference.md sync is unaffected), changelog complete (no release bump in this PR, consistent with PR description), error-codes clean, sentinel-guards clean, file-size gate OK (only pre-existing soft-ceiling warnings on unrelated files), 5874 passed, 12 skipped ✓
• Confirmed "storage.describe-batch": "write" already present in permissions.py OPERATION_REGISTRY (pre-existing, command signature unchanged, no new entry needed) ✓
• Confirmed storage describe-batch already listed in CLAUDE.md and commands/context.py AGENT_CONTEXT (unchanged, signature-compatible) ✓
• Live behavior reproduction against the isolated worktree build with the exact repro YAML from the issue (tables: as a list of objects):
  • --json mode: {"status": "error", "error": {"code": "INVALID_ARGUMENT", "message": "'tables' must be a mapping of table ID to description, got a list. Expected:\ntables:\n in.c-sales.orders: All sales orders", ...}}, exit 2 ✓ (matches PR description verbatim)
  • human mode: same message rendered as Error: ..., exit 2 ✓ (both output surfaces verified independently)
• Confirmed via kbagent storage describe-batch --help that the Typer docstring was NOT updated -> basis for [NB-1] ✓
• Checked gotchas.md for the (Release step: ... tag this sentence (since vX.Y.Z) ...) placeholder pattern used instead of an actual version tag -- found an identical pre-existing placeholder already on main at line 2693 for a different (already-merged, not-yet-released) gotcha, confirming this is an established repo convention for doc changes landing between releases, not a deviation introduced by this PR -- not flagged as a finding ✓
• Cross-checked all three updated doc surfaces (gotchas.md, storage-describe-workflow.md, commands-reference.md) for consistency: all three state the same contract (API failures partial-tolerant / shape failures fail-fast with INVALID_ARGUMENT + exit 2, nothing half-applied) with no contradictions between them ✓
• git diff main...HEAD --stat -- 11 new service-layer tests (TestDescribeBatchShapeValidation), 1 new CLI-layer test (test_describe_batch_malformed_shape_json_envelope, runs the real StorageService with a client factory that raises AssertionError if reached, proving the API is never touched on a malformed file); pre-existing E2E coverage for describe-batch already exists in tests/test_e2e.py:3979 from the command's original introduction -- no new E2E test needed for a pure input-validation fix with no new API surface ✓

Open questions for the author

(none)