feat(storage): table snapshots -- create/list/detail/delete + table-from-snapshot (0.75.0, #512)

[padak]

Closes #512.

What

kbagent could not create a table from an existing snapshot. This PR adds the full table-snapshot lifecycle as first-class storage commands (0.75.0):

Command	Purpose	Permission
storage snapshot-create	Point-in-time backup (data + columns + PK); receipt carries snapshot_id	write
storage snapshots	List a table's snapshots	read
storage snapshot-detail	Snapshot detail; embeds the source table object (global IDs trace back to their table)	read
storage table-from-snapshot	Create a NEW table from a snapshot (the core ask of #512)	write
storage snapshot-delete	Delete snapshots (batch-tolerant; forecloses restores, source tables untouched)	destructive

UX decision (the issue asked to confirm)

Dedicated commands, not a flag on create-table: restore goes through the classic tables-async endpoint with a snapshotId source, while create-table uses tables-definition, which does not accept snapshots. Endpoint contract verified against the reference PHP client (keboola/storage-api-php-client: createTableFromSnapshot, createTableSnapshot, listTableSnapshots, getSnapshot, deleteSnapshot).

Verified live on connection.us-east4.gcp.keboola.com (project 5946, 2026-07-22):

• --name is REQUIRED — the live API rejects an omitted/empty name (Table create option "name" is required and cannot be empty); the PHP client's "defaults to snapshot name" docblock is stale.
• No overwrite semantics — restoring onto an existing table name fails; the documented pattern is restore under a new name → verify → swap-tables/delete-table.
• Restored table matches the source exactly (rows, columns, primary key, isTyped).

Architecture

New modules (commands/storage.py and services/storage_service.py are past their CONTRIBUTING.md size ceilings — not propagating): services/snapshot_service.py, commands/_storage_snapshots.py (mounted flat onto storage_app, so permission keys stay in the storage.* namespace). Client methods in client.py (async ops poll via _wait_for_storage_job). kbagent serve gets the 1:1 routes (server/routers/storage.py) with SnapshotService wired into ServiceRegistry.

Ride-along E2E fixes (pre-existing, found while verifying)

• _test_mcp_parity_commands (added in feat: MCP parity commands + fail-closed tool firewall (0.73.0, epic #390 + #478 phase 0) #508) called _json() — which asserts exit 0 — on a command it expects to fail; the full suite could never pass step 38.5 since 0.73.0. Fixed to parse the error envelope directly.
• Step 40 asserted the add/delete-column result on the first table-detail read. Measured live: the column listing is read-after-DDL eventually consistent (~5–10 s lag, wider with recent snapshot activity on the table; reproduced twice in-suite + once standalone, self-resolving every time). New _poll_columns helper polls up to ~30 s on both the add and delete side.

With both fixes the full TestFullE2E suite passes end-to-end (5:07, config-dir mode against project 5946) — including new snapshot step 11.2. Note: the nightly E2E workflow has been red for 40+ runs for an unrelated reason (expired E2E_API_TOKEN repo secret; token 5946-...v0jX is invalid — needs a maintainer rotation).

Tests

• 30 unit tests (tests/test_storage_snapshots.py): HTTP shape via pytest_httpx (URLs, branch prefixes, job polling, 204-vs-202 delete), service logic (error accumulation, dry-run short-circuits, blank-input rejection, client close), CLI (CliRunner: JSON shapes, exit codes 1/2/5, confirm-abort, active-branch resolution).
• 5 router→service kwarg-parity tests (tests/test_server_router_calls.py).
• E2E step 11.2: create → list → detail → dry-run → restore → verify rows/columns/PK → cleanup.
• make check green (4648 passed).

Plugin / docs surfaces touched (per the CONTRIBUTING.md sync map)

• src/keboola_agent_cli/commands/context.py — new "Table Snapshots" section in AGENT_CONTEXT
• CLAUDE.md ## All CLI Commands
• plugins/kbagent/agents/keboola-expert.md — two §2 matrix rows + §3 inline gotcha (59.0 KB, under the 62 KB budget)
• plugins/kbagent/skills/kbagent/SKILL.md — regenerated table (make skill-gen) + description triggers (compressed to stay under the 1024-char Claude Desktop cap) + workflow-table row
• plugins/kbagent/skills/kbagent/references/commands-reference.md — five bullets (since v0.75.0)
• plugins/kbagent/skills/kbagent/references/gotchas.md — (since v0.75.0) section
• plugins/kbagent/skills/kbagent/references/snapshot-workflow.md — new workflow doc (backup → find → restore-as-new → swap-promote → cleanup)
• permissions.py OPERATION_REGISTRY (5 entries), changelog.py (0.75.0), plugin.json/marketplace.json via make version-sync

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

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

[padak]

Review of #516 — feat(storage): table snapshots -- create/list/detail/delete + table-from-snapshot (0.75.0, #512)

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 the full table-snapshot lifecycle (snapshot-create, snapshots, snapshot-detail, snapshot-delete, table-from-snapshot) as new kbagent storage commands, closing #512. Implementation is clean 3-layer: new client.py methods, a new SnapshotService, and a new thin commands/_storage_snapshots.py module (correctly split out because storage.py/storage_service.py are already past their CONTRIBUTING.md size ceilings). Every plugin-synchronization surface (AGENT_CONTEXT, CLAUDE.md, keboola-expert.md matrix + gotcha, SKILL.md, commands-reference.md, gotchas.md with version tag, new snapshot-workflow.md), OPERATION_REGISTRY, and the kbagent serve REST routes (with kwarg-parity tests) are all present. Test coverage is thorough (client/service/CLI unit tests + router-parity tests + a full E2E roundtrip), and make check passes clean (4648 passed, 8 skipped). Verdict: COMMENT — no blocking issues found; two non-blocking design nits worth the author's attention.

Verdict

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

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/services/snapshot_service.py:240 — new _resolve_project return is a heterogeneous 2-tuple

_resolve_project(self, alias: str) -> tuple[str, str] returns (stack_url, token) — two semantically-distinct strings (a URL and a secret credential) that a caller has to remember the positional order of (stack_url, token = self._resolve_project(alias), five call sites). Per CONTRIBUTING.md "Return values -- name them with dataclasses, not tuples", this is exactly the case that should be a small @dataclass(frozen=True) (e.g. ResolvedProject(stack_url: str, token: str)) rather than a bare tuple. This is a genuinely new tuple[...] return added by this diff (the ~63 pre-existing ones are grandfathered) — it happens to mirror the identical pattern already in token_service.py/stream_service.py (the module docstring says "mirrors the TokenService layout"), so fixing it in isolation here would create inconsistency with those two siblings. Worth a small follow-up across all three rather than blocking this PR.

[NB-2] src/keboola_agent_cli/client.py — new snapshot methods added to a file already ~2x over its hard LOC ceiling

client.py is 3962 lines against CONTRIBUTING.md's 2000-line hard ceiling for client.py/manage_client.py ("splitting is required before merging more functionality into it"). This PR adds ~120 lines of new snapshot methods on top of that. The PR correctly avoided growing commands/storage.py and services/storage_service.py further (both already flagged in the PR description as past-ceiling, with new sibling modules created instead) — the same discipline wasn't extended to client.py. This is pre-existing, systemic debt (the file was already ~3000 LOC per CONTRIBUTING.md's own historical note) rather than something this PR introduced, so not a merge blocker, but worth a follow-up issue to split client.py by endpoint family (client/storage.py, client/queue.py, ...) per the guidance already in CONTRIBUTING.md.

Nits

• [NIT-1] src/keboola_agent_cli/commands/_storage_snapshots.py:34 — _handle_errors(formatter: Any, exc: Exception) -> None deviates from the established -> NoReturn convention used by the identical helper in sibling modules (commands/feature.py:37, commands/token.py:39, commands/stream.py:46), all of which import NoReturn from typing. Functionally harmless (the function does always raise), but NoReturn gives static analysis better information and matches the pattern this module explicitly says it mirrors.
• [NIT-2] src/keboola_agent_cli/server/routers/storage.py:449 vs :465 — the create route nests under /tables/{project}/{table_id}/snapshots (POST) while the sibling list route is /snapshots/{project}/{table_id} (GET), not /tables/{project}/{table_id}/snapshots (GET). Minor asymmetry between the two closest-related routes; low value fix given the router file's existing naming is already loosely conventioned (table-detail, table-preview, etc. as flat top-level segments) and every route has an explicit kwarg-parity test regardless.

Verification log

• gh auth status → authenticated as padak, token scopes repo, workflow, ... ✓
• gh pr view 516 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → OPEN, 23 files, +2003/-30, base main, conventional prefix feat(storage): matches new-behavior change type ✓
• git rev-parse --abbrev-ref HEAD → feat/512-table-snapshots (matches <branch>; working tree already on the PR branch, no checkout performed) ✓
• gh pr diff 516 → 2350-line diff fetched to scratch file; gh pr view --json files → 23-file list fetched ✓
• 3-layer violation greps (typer/click in services; httpx/requests in commands; formatter/console.print/typer in clients) → all empty, no layer violations ✓
• Manual read of client.py, commands/_storage_snapshots.py, services/snapshot_service.py, permissions.py, cli.py, server/routers/storage.py, server/dependencies.py diffs → commands are thin (parse → service call → format/error-map), service does all business logic + validation (blank-input rejection, project resolution), client does all HTTP + job polling ✓
• permissions.py OPERATION_REGISTRY → all 5 new commands present with correct categories (snapshots/snapshot-detail=read, snapshot-create/table-from-snapshot=write, snapshot-delete=destructive), matching CONTRIBUTING.md's read/write/destructive definitions ✓
• Plugin synchronization map walk: commands/context.py (new "Table Snapshots" section, 5 commands) ✓, CLAUDE.md ## All CLI Commands (5 lines + inline note) ✓, keboola-expert.md §2 (2 new matrix rows) + §3 (1 new gotcha, tagged 0.75.0) ✓, SKILL.md decision table (5 rows) + description triggers + workflow-table row ✓, commands-reference.md (5 bullets, each (since v0.75.0)) ✓, gotchas.md (new section tagged (since v0.75.0)) ✓, new snapshot-workflow.md (104 lines) ✓, server/routers/storage.py 1:1 REST mirror (5 routes) + server/dependencies.py ServiceRegistry wiring ✓ — all "NO" (silent-drift) rows from the sync map are covered
• wc -c plugins/kbagent/agents/keboola-expert.md → 59002 bytes, under the actual PROMPT_BYTE_BUDGET = 62_000 enforced by tests/test_agent_prompt.py (PR description's "under the 62 KB budget" claim confirmed against the real test threshold, not the slightly-stale 60 KB figure in CLAUDE.md's own prose) ✓
• make check → ruff check/ruff format --check clean; ty check reports 3 pre-existing warnings unrelated to this diff (scripts/hatch_build.py, tests/test_mcp_deprecation_warnings.py, tests/test_tool_call_permissions.py — none touched by this PR; downgraded unresolved-import rule, non-blocking per CONTRIBUTING.md); SKILL.md is up-to-date; version is in sync; check_command_sync.py → "OK: all 251 CLI commands are registered (OPERATION_REGISTRY) and documented"; generate_changelog.py --check → all 43 stable releases have entries; check_error_codes.py → OK, no raw literals; full suite → 4648 passed, 8 skipped (exact match to the PR description's claimed count) ✓
• uv run kbagent storage --help → new "Snapshots" rich-help panel lists all 5 commands with correct one-line summaries ✓
• uv run kbagent storage table-from-snapshot --help / snapshot-delete --help → option help text renders correctly, matches docs (--name required, --dry-run/--yes present on delete) ✓
• uv run python -c "... app.openapi() ..." → confirmed all 5 /storage/*snapshot* REST routes are registered on the FastAPI app (POST /storage/tables/{p}/{t}/snapshots, GET /storage/snapshots/{p}/{t}, GET /storage/snapshot-detail/{p}/{id}, DELETE /storage/snapshots/{p}, POST /storage/table-from-snapshot/{p}) ✓
• wc -l on new/touched files → commands/_storage_snapshots.py 299 LOC (soft 800/hard 1200), services/snapshot_service.py 247 LOC (soft 1000/hard 1500) — both well within budget; client.py 3962 LOC vs hard ceiling 2000 (pre-existing over-ceiling file, PR adds ~120 lines to it, see NB-2) ✓
• Convention greps (magic numbers, raw error_code="..." strings, bare except:, print() in prod code, token leakage in new log/print statements) → all empty/false-positive-only (token hits are legitimate constructor params, never logged) ✓
• New -> tuple[...] annotations in the diff → exactly one: snapshot_service.py:240 (see NB-1); no others ✓
• Could not reproduce the PR's live-API claims myself: this worktree has no registered kbagent config-dir / project token, and per team convention the reviewer never handles or provisions API tokens. The PR description documents specific live verification (project 5946, connection.us-east4.gcp.keboola.com, 2026-07-22, with the exact API error string for the missing---name case) — I could not independently confirm this against a live stack, but the 30 unit tests encode matching request/response shapes via pytest_httpx (exact URLs, request bodies, 202-vs-204 delete branching, job-polling), which is internally consistent with the documented API contract.

Open questions for the author

(none)

[padak]

Self-review findings disposition (author):

• NIT-1 (_handle_errors → NoReturn): fixed in c44876b.
• NB-1 (_resolve_project tuple return): agreed — but fixing it only here would diverge from the identical grandfathered pattern in token_service.py / stream_service.py, as the review itself notes. Deferred to a follow-up converting all three to a shared frozen dataclass.
• NB-2 (client.py over the hard LOC ceiling): pre-existing systemic debt; a follow-up issue proposing the endpoint-family split per CONTRIBUTING.md is being prepared.
• NIT-2 (route naming asymmetry): leaving as-is — the router's existing convention is already flat-segment based (table-detail, table-preview), and each route carries a kwarg-parity test.

[padak]

Follow-ups from this review are now tracked:

• NB-1 (_resolve_project tuple → dataclass, all three services): PR refactor(services): ResolvedProjectCredentials dataclass over (stack_url, token) tuple (#516 NB-1) #519 (stacked on this branch; merges after feat(storage): table snapshots -- create/list/detail/delete + table-from-snapshot (0.75.0, #512) #516).
• NB-2 (client.py 3,842 LOC vs 2,000 hard ceiling): issue Split client.py by endpoint family — 3,842 LOC vs 2,000 hard ceiling (PR #516 NB-2) #520 with a grep-measured endpoint-family inventory and an incremental, .raw-preserving mixin split plan.