fix(AI-3757): forward authorization/runtime in component sync-action - #620
Conversation
ComponentService.run_sync_action built configData from only parameters and storage, dropping the root configuration's authorization block. For OAuth/Service-Account components (e.g. keboola.ex-linkedin-ads) that block carries the broker reference the sync-actions service needs to resolve credentials, so the component crashed before its own error handling ran, surfacing as an opaque empty-body 400 (SUPPORT-17393). Mirrors keboola-mcp-server's run_sync_action: authorization and runtime are read from the root config only (rows never override them) and forwarded when non-empty. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@claude review |
There was a problem hiding this comment.
Pull request overview
This PR fixes ComponentService.run_sync_action so the kbagent component sync-action path forwards the root configuration’s runtime and authorization blocks (when present) into the Sync Actions configData payload—aligning behavior with Keboola UI and restoring functionality for OAuth/service-account components that rely on broker references (AI-3757 / SUPPORT-17393).
Changes:
- Extend
configDataconstruction to include root-onlyruntimeandauthorizationwhen non-empty (rows still only shallow-mergeparameters/storage). - Add service-level tests covering forwarding behavior and omission when absent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/keboola_agent_cli/services/component_service.py |
Includes root runtime/authorization in sync-action configData when present, without changing override semantics. |
tests/test_component_sync_action.py |
Updates root-config fixture and adds targeted regression tests for the new forwarding behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
padak
left a comment
There was a problem hiding this comment.
Review of #620 — fix(AI-3757): forward authorization/runtime in component sync-action
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This is a small, well-scoped bug fix to services/component_service.py::run_sync_action: it now forwards the root configuration's runtime and authorization blocks (when non-empty) into the configData payload sent to the Sync Actions API, matching keboola-mcp-server's run_sync_action implementation (verified byte-for-byte against upstream keboola/mcp-server source, including the "docker-runner contract" comment). Without this, OAuth/Service-Account components like keboola.ex-linkedin-ads crashed before their own error handling ran, surfacing as an opaque empty-body 400. Verdict: COMMENT — the fix itself is correct, well-tested, and verified against the reference implementation; the only gap is a missing gotchas.md entry for a documented-command behavior change, which is a NON-BLOCKING drift risk rather than a functional defect.
Verdict
- Verdict: COMMENT
- Blocking findings: 0
- Non-blocking findings: 2
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] plugins/kbagent/skills/kbagent/references/gotchas.md:273 — missing gotcha entry for the authorization/runtime forwarding fix
The gotchas file already has a dedicated bullet for component sync-action --row-id merge semantics right next to where this change belongs (line 273, "Native parity commands" section). This PR changes the actual wire behavior of a documented command (OAuth/Service-Account sync actions like keboola.ex-linkedin-ads list_accounts now succeed instead of failing with an opaque empty-body 400) — exactly the class of non-obvious, version-dependent behavior CONTRIBUTING.md's "Plugin synchronization map" flags as mandatory to log with a (since vX.Y.Z) tag, so an AI agent troubleshooting the same symptom on an older install knows to tell the user to upgrade rather than assume the CLI is broken by design.
Fix: add a bullet under the "Native parity commands" section, e.g. "component sync-action forwards root runtime/authorization (rows never override them) since vX.Y.Z — earlier versions silently dropped authorization, breaking OAuth/Service-Account components with an opaque empty-body 400." Tag with whatever version this PR ships in.
[NB-2] plugins/kbagent/skills/kbagent/references/commands-reference.md:119 — sync-action entry doesn't mention runtime/authorization forwarding
The existing bullet documents the --row-id shallow-merge in detail but says nothing about runtime/authorization being forwarded from root. Not a new command or flag (so not strictly required by the Plugin synchronization map's "flag changes" trigger), but since this file is the CLI's hand-maintained per-command reference and the fix is user-visible (previously-failing OAuth actions now work), a one-clause addition would help.
Fix: append something like "root runtime/authorization are also forwarded (rows never override them) -- needed for OAuth/Service-Account components."
Nits
[NIT-1]PR description ends with a "🤖 Generated with Claude Code" footer, whichCONTRIBUTING.md> "Commit & PR Conventions" explicitly forbids ("No AI attribution footers in PR descriptions"). Worth stripping before merge for consistency with repo policy.
Verification log
gh pr view 620 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ OPEN,fix(AI-3757):prefix (matches bug-fix nature), 2 files changed (+78/-5):services/component_service.py,tests/test_component_sync_action.py. ✓ description accurately matches the diff (no undisclosed scope creep).gh pr diff 620→ 130-line diff, entirely within LAYER 2 (services/) plus tests. Nocommands/,client/,permissions.py,server/routers/touched — correct, since this fixes payload construction inside an already-registered command (component sync-action), not a new command.- Layer-violation greps (typer/click in services, httpx in commands, formatter/typer in clients) on the diff → all empty ✓
- Convention greps (magic numbers, raw
error_codestrings, bareexcept:,print(), newtuple[...]returns, token leakage) on the diff → all empty ✓ git show pr-620-review:.../client/misc.py(run_sync_action) → confirmsconfig_datais forwarded verbatim asconfigDatain the POST body; no client-layer change needed. ✓- Cross-checked against upstream
keboola/mcp-serversource (gh api repos/keboola/mcp-server/contents/src/keboola_mcp_server/tools/components/tools.py) → the CLI's fix is functionally and comment-for-comment identical to MCP'srun_sync_action(runtime/authorizationpulled from root only, forwarded only when non-empty, same "docker-runner contract" rationale). PR's claim of MCP parity is verified, not just asserted. ✓ src/keboola_agent_cli/permissions.py→"component.sync-action": "write"already registered (pre-existing; unaffected by this fix). ✓- Isolated detached worktree (
git worktree add --detach /tmp/kbagent-review-pr620 pr-620-review, invoking worktree untouched) +uv sync --extra server:uv run ruff check src/ tests/ scripts/→ All checks passed ✓uv run ruff format . --check→ 433 files already formatted ✓uv run ty check→ 1 pre-existing unrelated warning (scripts/hatch_build.py, hatchling stub resolution), 0 diagnostics touching this PR's files ✓SKILL.mdup-to-date, version in sync ✓scripts/check_command_sync.py→ OK: all 261 CLI commands registered/documented (confirms no command-surface drift from this PR) ✓scripts/generate_changelog.py --check→ FAILS ("Missing changelog entries for: 0.88.0, 0.87.0") — traced to the PR branch being 17 commits / 2 releases behind currentmain(git merge-base= the 0.86.0 release commit), not caused by this PR's diff.gh pr checks 620confirms CI is green (CI does not runchangelog-check, per repo convention — it's a local-onlymake checkstep). Author should rebase before merge to avoid a stale-branch surprise, but this is not a defect in the PR itself.scripts/check_error_codes.py→ OK: no raw error_code literals ✓scripts/check_sentinel_guards.py→ OK: no unguarded credential writes/clients ✓uv run pytest tests/test_component_sync_action.py tests/test_component_service.py tests/test_cli.py -q→ 293 passed ✓uv run pytest tests/ -q(full suite) → 5693 passed, 172 skipped ✓
gh pr checks 620→ all 4 checks pass (Windows wheel build, check, test 3.12, test 3.13) ✓
Open questions for the author
(none)
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.
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.
…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.
Summary
kbagent component sync-actionbuilds itsconfigDatapayload from only the config'sparametersandstorage, dropping the root configuration'sauthorization(andruntime) block.keboola.ex-linkedin-ads'slist_accounts— that block carries the broker reference the sync-actions service needs to resolve/decrypt credentials before invoking the component. Without it, the component crashes before its own error-handling wrapper runs, surfacing as an opaque, empty-body400({"error": "", "code": 0, ...}) instead of a clean error. The same action succeeds via the Keboola UI, which does forward it.keboola-mcp-server'srun_sync_action(the tool this command was ported from for MCP parity):runtimeandauthorizationare read from the ROOT configuration only (rows never override them) and forwarded only when non-empty.Test plan
uv run pytest tests/test_component_sync_action.py -v— 24 passed, including two new cases:test_authorization_and_runtime_forwarded_from_rootandtest_authorization_and_runtime_omitted_when_absentuv run pytest tests/test_component_service.py tests/test_cli.py -q— 269 passed, no regressionsruff check/ruff format --check/ty checkon changed files — all cleanRelated issues
Fixes CLI-1 (SUPPORT-17393).
🤖 Generated with Claude Code