fix(config): never purge on a repeated delete; add restore + trash-list (0.89.0) - #643
Merged
Conversation
…st (0.89.0)
The Storage API overloads DELETE .../configs/{id}: on a live configuration
it soft-deletes into the trash, but on a configuration ALREADY in the trash
the same call purges it permanently -- versions, rows and metadata included.
A timed-out delete followed by a retry is exactly that second call, and
retrying on timeout is what every agent and CI script does.
`config delete` now locates the configuration before deleting:
* live -> DELETE, status "deleted"
* already in the trash -> NO second DELETE, status "already_in_trash",
exit 0 (the retry stays idempotent for scripts)
* absent from both -> NOT_FOUND (a plain GET 404s for trashed AND missing
configs alike, so the trash listing is what separates the two)
* --dry-run reports the located state without writing
New commands complete the loop:
* `config restore` -- POST .../configs/{id}/restore, the undo (versions,
rows and metadata come back); permission class write
* `config trash-list` -- what restore can bring back, project-wide or per
component; permission class read
All three are mirrored on `kbagent serve` (DELETE gains dry_run;
POST .../restore and GET /configs/trash/{project} are new).
Docs: CLAUDE.md's All CLI Commands had NEVER listed `config delete` --
silent drift that made the command look nonexistent to AI agents reading
the file (it misled one today). Added, along with the double-delete gotcha
in gotchas.md and entries in commands-reference.md and AGENT_CONTEXT.
Endpoint semantics verified against keboola/connection source: the official
PHP client's purgeConfiguration docblock states the repeated-DELETE purge
behaviour and offers POST .../purge (400 when not trashed) as the safe
explicit path; restoreComponentConfiguration confirms the restore route.
Trash lookup and result shaping live in services/_config_trash.py because
config_service.py is over its size budget; the service methods stay thin.
Commands live in commands/_config_trash_cmd.py for the same reason
(config.py), mounted via register() like _config_clone_cmd.py.
15 unit tests (the assert_not_called() checks on client.delete_config are
the point: they prove the purge call cannot happen) + 3 router tests. The
E2E delete step now runs the full round trip live: delete -> repeated
delete answers already_in_trash -> trash-list finds it -> restore ->
detail confirms live -> final delete.
Version 0.89.0 (pyproject + version-sync), changelog block added -- which
is also what satisfies check_version_gates for the new (since v0.89.0)
markers; the gate caught this PR's own docs before the changelog existed.
The first commit pushed services/config_service.py from 1487 to 1566 code lines, over the 1500 HARD ceiling -- make loc-check rightly blocked it. - the delete/restore/trash-list bodies move fully into services/_config_trash.py (execute_delete / execute_restore / execute_trash_list); the ConfigService methods are resolve-and-delegate only - _find_matches_in_json moves to json_utils.py as find_matches_in_json -- it is a pure JSON-walking helper and json_utils is where those live (deep_merge, set_nested_value, compute_diff); the one call site and the test import follow config_service.py lands at 1493 code lines: under the hard ceiling, still carrying the pre-existing soft warning that a real split is due.
… a purge Devin Review caught a hole in the first commit's central claim. The locate-first guard runs ONCE per delete_config call, so it protects against a retry across separate command invocations -- but not against the retry the HTTP client performs inside that single call. DELETE is in RETRY_SAFE_METHODS, so a read timeout or a 5xx made _do_request repeat it automatically. On this endpoint the repeat IS the purge: the server trashes the config, the response is lost, the retry lands on the now-trashed config and destroys it before any caller sees a result. That is the MOST likely form of the very scenario the PR set out to close, and it was still open. Reproduced before fixing: one delete_config call put two DELETEs on the wire. - http_base._do_request gains a per-call `retry_safe` override; None keeps the method-based rule. _server_error_hint honours it too, so a 5xx on an opted-out call gets the "may already have taken effect" note. - KeboolaClient._request passes it through; client.delete_config sets retry_safe=False. A lost response now surfaces as TIMEOUT for the caller to decide about, and re-running the command is safe because the service guard catches the trashed state -- which a transport-level retry never can. - Every other DELETE keeps its retry. The opt-out is per endpoint because idempotency is a property of the endpoint, not of the method. Second Devin point, also addressed: locate_config inferred "live" from the absence of a 404. GET on a trashed config answers 404 on connection.keboola.com and on the GCP stack (verified live), but a stack returning the tombstone body would have made a 200 mean "trashed" -- and the DELETE that followed would purge. It now reads `isDeleted` from the body rather than trusting the status code, so the behaviour no longer depends on a cross-stack convention. 5 tests added (20 in the file): both purge paths register exactly ONE mocked outcome, so a passing test proves a single DELETE left the client; plus the default-still-retries case and the override in isolation.
padak
force-pushed
the
feat/config-delete-restore
branch
from
August 22, 2026 21:00
caaee67 to
01a364e
Compare
padak
added a commit
that referenced
this pull request
Aug 22, 2026
0.89.0 (config delete trash guard, #643) is merged on main but not yet published, and changelog-check allows exactly one in-flight version -- so the #646 entry joins the 0.89.0 release being prepared instead of stacking a second unreleased bump on top. pyproject back to 0.89.0 (version-sync'd), docs tags now read (since v0.89.0).
This was referenced Aug 22, 2026
padak
added a commit
that referenced
this pull request
Aug 22, 2026
pyproject/plugin.json/marketplace.json were already renumbered to 0.89.0 by v0.88.0. Changelog: adds 0.89.0 entries for #645 (describe-batch --from-file shape validation, issue #640), #642 (table-detail human column descriptions), #620 (sync-action forwards root authorization/runtime), #517 (stable metavar contract, issue #513), #586 (documented prompt budget gated against the enforced one, issue #585) and #641 (docs-only), and decorates the existing Silent-drift surfaces: * gotchas.md -- resolves both "(Release step: ... tag this sentence)" placeholders. Both were left by commits AFTER the v0.88.0 tag (#642 and #645), so both are tagged (since v0.89.0), not 0.88.0. Adds the #620 gotcha: below 0.89.0 a sync action on an OAuth / Service-Account component died with an opaque empty-body 400 because the broker reference was never forwarded. * #620 shipped with no doc surfaces at all -- CLAUDE.md, AGENT_CONTEXT and commands-reference.md now carry the forwarding rule (root only, never row-overridden, only when non-empty) with its version gate. * #645 never reached CLAUDE.md -- the describe-batch shape check and its behaviour change are recorded there now; commands-reference gains the version tag. * #642's human Description column is version-tagged in CLAUDE.md, commands-reference.md, AGENT_CONTEXT and storage-describe-workflow.md. * #643 was otherwise complete; adds the two surfaces it did not touch -- safe-write-workflow.md (delete is reversible; never blind-retry on <= 0.88.x) and a keboola-expert.md matrix row for delete/restore/trash-list. keboola-expert.md is 49 774 B, well inside the 70 000 B budget. make check green: 5934 passed, 12 skipped. version-gate-check resolves all 438 markers across 72 versions.
padak
added a commit
that referenced
this pull request
Aug 23, 2026
pyproject/plugin.json/marketplace.json were already renumbered to 0.89.0 by v0.88.0. Changelog: adds 0.89.0 entries for #645 (describe-batch --from-file shape validation, issue #640), #642 (table-detail human column descriptions), #620 (sync-action forwards root authorization/runtime), #517 (stable metavar contract, issue #513), #586 (documented prompt budget gated against the enforced one, issue #585) and #641 (docs-only), and decorates the existing Silent-drift surfaces: * gotchas.md -- resolves both "(Release step: ... tag this sentence)" placeholders. Both were left by commits AFTER the v0.88.0 tag (#642 and #645), so both are tagged (since v0.89.0), not 0.88.0. Adds the #620 gotcha: below 0.89.0 a sync action on an OAuth / Service-Account component died with an opaque empty-body 400 because the broker reference was never forwarded. * #620 shipped with no doc surfaces at all -- CLAUDE.md, AGENT_CONTEXT and commands-reference.md now carry the forwarding rule (root only, never row-overridden, only when non-empty) with its version gate. * #645 never reached CLAUDE.md -- the describe-batch shape check and its behaviour change are recorded there now; commands-reference gains the version tag. * #642's human Description column is version-tagged in CLAUDE.md, commands-reference.md, AGENT_CONTEXT and storage-describe-workflow.md. * #643 was otherwise complete; adds the two surfaces it did not touch -- safe-write-workflow.md (delete is reversible; never blind-retry on <= 0.88.x) and a keboola-expert.md matrix row for delete/restore/trash-list. keboola-expert.md is 49 774 B, well inside the 70 000 B budget. make check green: 5934 passed, 12 skipped. version-gate-check resolves all 438 markers across 72 versions.
padak
added a commit
that referenced
this pull request
Aug 23, 2026
…es (#651) Release prep for 0.89.0: adds the changelog entries for everything merged since v0.88.0 (#620, #642, #643, #644, #645, #646, #647, #648, #649, #650, #517, #586, #641), resolves every vNEXT placeholder left by feature PRs to v0.89.0 per the new #648 release process, closes the 10 gaps a full doc-surface audit found across the kbagent plugin (SKILL.md triggers, commands-reference, gotchas, workflow files, keboola-expert.md, AGENT_CONTEXT, CLAUDE.md), and records the live e2e verification evidence. Version files were already at 0.89.0 (bumped by #643); make version-sync is a no-op.
This was referenced Aug 23, 2026
docs: web-server.md is substantially stale — 10 routers and every 0.89.0 serve addition missing
#656
Closed
padak
added a commit
that referenced
this pull request
Aug 23, 2026
…ens, palette (#658) NERD web UI catch-up to the 0.89.0 feature set, frontend + docs only. Jobs gain re-run (branch-preserving, config-aware) and terminate (gated to terminable statuses, ConfirmModal); config detail moves to a Drawer with Run job; configs get delete + a Trash tab with restore (#643); new Tokens page (list, opt-in derive-last-used with dormant-first sort, create/rotate/delete, secret shown once); Storage table drawer renders the BigQuery definition (#621) and column descriptions are click-to-edit (#624); Dashboard gains a PAYG credits tile; Flows gain a read-only Notifications tab (project-wide catch-alls grouped separately); ctrl/cmd+K command palette over pages, projects and actions. Fixes two long-standing UI bugs along the way: the jobs Config column read an invented configId key (API sends config — CLI twin tracked in #659) and ConfirmModal now portals to body. Cleanups: window.confirm removed, dead useManageTokenPrompt deleted. Frontend CI gate gap tracked in #660.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The trap
The Storage API overloads
DELETE .../configs/{id}: on a live configuration it soft-deletes into the trash, but on a configuration already in the trash the same call purges it permanently — versions, rows and metadata included. Verified againstkeboola/connectionsource: the official PHP client'spurgeConfigurationdocblock states it outright ("Unlike a repeated deleteConfiguration() call, this fails with 400 when the configuration is not in the trash, so a stale caller cannot destroy a live configuration") — the dedicated purge endpoint exists precisely because a repeated DELETE destroys.A timed-out delete followed by a retry is exactly that second call. Retrying on timeout is what every agent and every CI script does. kbagent's goal is to be the CLI agents can be trusted with, so the CLI is where this gets closed.
What changed
config deletelocates the configuration before deleting:status: "deleted"status: "already_in_trash", exit 0 — the retry stays idempotentThe lookup needs two probes because a plain
GET .../configs/{id}answers 404 for a trashed config and a missing one alike — only theisDeleted=truelisting separates them.--dry-runreports the located state without writing.Two new commands complete the loop:
config restore—POST .../configs/{id}/restore, the undo; versions, rows and metadata come back (write)config trash-list— what restore can bring back, project-wide or--component-id-scoped (read)All three mirrored on
kbagent serve:DELETEgainsdry_run;POST .../restoreandGET /configs/trash/{project}are new.The documentation finding
CLAUDE.md's All CLI Commands section had never listedconfig delete— the command existed for a long time, the inventory just omitted it. That is convention #17's silent drift in its purest form, and it misled an AI agent (me) earlier today into concluding the command didn't exist and advising deletion via the UI. Fixed here along with the double-delete gotcha ingotchas.md,commands-reference.mdentries andAGENT_CONTEXT.Structure
services/_config_trash.py(config_service.py is over its size budget; the service methods stay thin — docstrings are free per loc-check)commands/_config_trash_cmd.py, mounted viaregister()like_config_clone_cmd.pyReview round 2 — the hole Devin found in the above
The locate-first guard runs once per call, so it closes a retry across separate command invocations — but not the retry the HTTP client performs inside that one call.
DELETEis inRETRY_SAFE_METHODS, so a read timeout or a 5xx made_do_requestrepeat it automatically, and on this endpoint the automatic repeat is the purge. That is the most likely form of the very scenario this PR set out to close, and the first commit left it open.Reproduced before fixing: one
delete_configcall put two DELETEs on the wire._do_requestgains a per-callretry_safeoverride (Nonekeeps the method rule);client.delete_configsetsFalse. A lost response now surfaces as TIMEOUT for the caller to decide about, and the re-run is safe because the service guard catches the trashed state — which a transport retry never can.locate_configno longer infers "live" from the absence of a 404: a body carryingisDeleted: trueis read as trashed regardless of status code. (404-on-trashed is confirmed live on AWS and GCP stacks, but that flag decides whether a purge-capable DELETE goes out, so it is read rather than inferred.)Tests
client.delete_config.assert_not_called()assertions are the point of the file: they prove the purge call cannot happen for trashed/missing/dry-run/500-on-preflight pathsalready_in_trash→trash-listfinds it →restore→config detailconfirms live → final delete. This proves the guard against the real endpoint that overloads DELETE, not a mock.Version
0.89.0 (pyproject + version-sync + changelog block). Pleasing detail:
check_version_gates— added in #639 — failed this PR's own docs the moment(since v0.89.0)markers landed before the changelog block existed. First real catch.make checkgreen: 5875 passed, 12 skipped.Open question (not in this PR)
config deletehas no--yesconfirmation whileconfig row-deletedoes. Adding a TTY prompt now would break existing automation (the E2E suite itself calls it bare), so I left the semantics alone — worth a separate decision.