feat(0.44.0): kbagent agent <verb> -- CLI parity for /agents REST surface - #310
Conversation
081d59b to
f9c77f5
Compare
padak
left a comment
There was a problem hiding this comment.
Review of #310 — feat(0.42.0): kbagent agent <verb> — CLI parity for /agents REST surface
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
The PR adds twelve kbagent agent <verb> subcommands matching the /agents
REST surface, a --beta pre-release channel for kbagent version/update,
a Semantic Layer web UI page (Phases 1-3), error message hardening in
http_base.py, and a heuristic_generate_model field-type normalizer. The
core agent CLI work is well-structured and passes make check (3 339 tests).
The main concerns are: the Semantic Layer UI content (2 700+ LOC across 7
files) is entirely duplicated from the already-open PR #308 and is not
mentioned anywhere in the PR description — this creates a merge conflict risk
and makes the review surface much larger than the stated scope. Five of the
twelve new agent subcommands (show, run-detail, run-events, test,
prompt-improve) have no CliRunner test. agent prompt-improve has no
blocking REST endpoint but CONTRIBUTING.md requires a 1:1 CLI-to-route
mapping without a documented skip justification in this PR. These are
blocking in aggregate because a merge of both PR #308 and PR #310 will
produce a conflict on three shared files, and because the PR description
misrepresents the test coverage. Verdict: REQUEST CHANGES.
Verdict
- Verdict: REQUEST CHANGES
- Blocking findings: 3
- Non-blocking findings: 4
- Nits: 2
Blocking findings
[B-1] web/frontend/src/pages/SemanticLayer.tsx:1-1325, web/frontend/src/pages/SemanticLayerDialogs.tsx:1-1385 — Semantic Layer UI (2 710 LOC) is already open in PR #308 and not mentioned in this PR description
All seven web/backend files added by the two feat(semantic-layer-ui) commits
(SemanticLayer.tsx, SemanticLayerDialogs.tsx, App.tsx, Sidebar.tsx,
state.tsx, http_base.py, _semantic_layer_internals.py) are identical to
the files in the separate open PR #308 ("Semantic Layer UI — full parity with
kbagent CLI (4 phases)", branch feat/semantic-layer-ui). Merging both PRs to
main will produce a three-way conflict on those files. The PR description and
title make zero mention of the Semantic Layer UI, so reviewers and CI cannot
reason about its correctness as part of this PR.
Fix: either rebase this branch on top of feat/semantic-layer-ui so the
semantic-layer commits are shared, or remove those commits from this PR and
let PR #308 land first. Do not merge both PRs independently — they will
conflict.
[B-2] tests/test_agent_cli.py — five of twelve agent subcommands have no CliRunner test
The PR description states "13 CLI tests in test_agent_cli.py (every subcommand
via CliRunner)". Inspecting the file, these subcommands have no test function:
agent show, agent run-detail, agent run-events, agent test, and
agent prompt-improve. Per CONTRIBUTING.md § Tests: "CLI-layer tests — use
CliRunner, test JSON output, error exit codes". The claim of complete coverage
is inaccurate and the missing tests leave five code paths unverified at the CLI
layer (including the --no-stream branch of prompt-improve and the
--stream path of agent test).
Fix: add at minimum a happy-path CliRunner test for each missing subcommand
(human mode + --json mode where output differs).
[B-3] src/keboola_agent_cli/server/routers/agents.py:467 — agent prompt-improve has no blocking REST endpoint; CONTRIBUTING.md skip not documented in PR
CONTRIBUTING.md § "HTTP API endpoint" requires a 1:1 match between CLI
commands and server/routers/ endpoints: "every command in a group has a
matching endpoint... Skip allowed only for genuinely terminal-only commands
(interactive prompts, Rich-rendered output...). Document any skip in the PR
description with a one-line reason." The router ships only POST /agents/prompt/improve/stream (SSE). There is no blocking POST /agents/prompt/improve that would serve the --no-stream path used by
automation consumers. The PR description does not mention this skip.
Fix: either add a blocking POST /agents/prompt/improve endpoint that
collects the stream internally and returns the final done event's
data.prompt, or explicitly document in the PR description why this endpoint
is intentionally SSE-only (e.g. "LLM streaming makes a blocking variant
impractical; callers use the SSE form or kbagent agent prompt-improve --no-stream which drains the stream in-process"). The skip justification
must appear in the PR body so reviewers and the audit trail can verify it.
Non-blocking findings
[NB-1] src/keboola_agent_cli/commands/context.py:1016,1022 — kbagent version [--beta] and kbagent update [--beta] in AGENT_CONTEXT, but not in keboola-expert.md Rule 6 VERSION GATE
The --beta flag and KBAGENT_INCLUDE_PRERELEASE env var were added in
0.42.0 (commit c3efbba). context.py documents both. However, the Rule 6
VERSION GATE in keboola-expert.md mentions kbagent agent <verb> needs
0.42.0+ but does not mention kbagent update --beta / kbagent version --beta. The subagent will not know that --beta requires 0.42.0+ and may
recommend it on older installs. Per CONTRIBUTING.md: "VERSION GATE examples
when adding a command that introduces a minimum-version requirement."
Fix: add one line to the Rule 6 VERSION GATE block:
kbagent update --beta / kbagent version --beta (pre-release opt-in) needs 0.42.0+.
[NB-2] tests/test_agent_cli.py — agent test command not covered by CLI tests at all (distinct from B-2 scope)
While B-2 covers the absence of several subcommand tests, agent test is
particularly important to call out: it exercises _NullStore (the in-memory
no-op store that suppresses disk writes) and the AgentService.stream_test
dispatch path. These code paths are only exercised via service-layer tests
(test_agent_service.py::test_stream_cli_command_yields_init_then_done) but
the CLI integration path (CliRunner invoking agent test --type cli_command --argv version) is missing. The service mock in test_agent_cli.py would
catch CLI-layer bugs that the service test cannot see (e.g. the --type
parameter plumbing, --stream/--no-stream flag routing, JSON output shape).
Fix: add TestAgentTest class with at least one CliRunner test using
--type cli_command --argv version.
[NB-3] src/keboola_agent_cli/services/agent_service.py:223,380 — default limit (50) and count (5) are inline literals, not constants
list_runs(... limit: int = 50) and cron_preview(... count: int = 5) use
inline integer defaults. CONTRIBUTING.md § "Constants — no magic numbers":
"All configuration values go in constants.py". The defaults 50 and 5 are
observable API contract (callers of the service or the CLI flag defaults match
them), so having them as literals means a change in one place does not
propagate.
Fix: add AGENT_DEFAULT_RUNS_LIMIT: int = 50 and AGENT_DEFAULT_CRON_PREVIEW_COUNT: int = 5
to constants.py and reference them in both the service defaults and the Typer
option defaults.
[NB-4] tests/test_agent_cli.py — E2E test for agent prompt-improve absent; behavior cannot be verified without live AI CLI
The three E2E tests in TestE2EAgentTasks cover cron-preview, full
create-show-update-run-runs-delete lifecycle, and ad-hoc agent test. They
do not cover agent prompt-improve. Unlike the other subcommands, this one
spawns an external AI CLI process (claude/codex/gemini), making offline unit
testing impossible and making an E2E test the only realistic way to validate
the end-to-end dispatch. CONTRIBUTING.md § E2E tests: "every CLI command MUST
have E2E coverage." An E2E test with --type cli_command (not an actual AI
CLI) would at least cover the dispatch plumbing even if it cannot test the
prompt-polishing logic.
Fix: add test_agent_prompt_improve_cli_command to TestE2EAgentTasks
invoking kbagent --json agent prompt-improve --goal "say hello" --cli echo --no-stream (or equivalent substitution that does not require a real AI
subscription). Mark with @pytest.mark.skip(reason="requires live AI CLI")
for the full AI path.
Nits
-
[NIT-1]src/keboola_agent_cli/http_base.py:226— the comment block above the new
err_fieldextraction is duplicated: "Real Keboola APIs answer with one of
these keys in priority order." appears verbatim twice (lines 226 and 232 in
context), apparently from a copy-paste during the revision. One of the two
comment blocks can be removed. -
[NIT-2]src/keboola_agent_cli/server/agents_store.py:46— the docstring for
AgentTask.cronuses"0 * * * *"as the inline default with comment "every
hour, top of the hour". The same default also appears inAgentService.create
as an implicit fallback. If the default ever changes, it must be updated in
two places. Consider extractingDEFAULT_AGENT_CRON: str = "0 * * * *"to
constants.pyfor single-source consistency.
Verification log
gh auth status→ authenticated to github.com aspadak(gho_****) ✓gh pr view 310 --json state→"OPEN"✓ / 6 505 additions, 824 deletions, 37 files ✓CONTRIBUTING.md,CLAUDE.md,plugins/kbagent/agents/keboola-expert.md→ all present and readable ✓git rev-parse --abbrev-ref HEADin worktree →feat/agent-cli-parity✓ (matches<branch>input)gh pr diff 310→ 7 986-line diff saved to/tmp/kbagent-pr-310.diff✓- Layer violation scan (typer in services, httpx in commands, formatter in clients) → empty ✓ (no violations; the
import typeradditions are incommands/agent.pyandcommands/version.py— correct layer) src/keboola_agent_cli/permissions.pyOPERATION_REGISTRY→ 12 newagent.*entries confirmed (lines 97–110) ✓src/keboola_agent_cli/hints/definitions/agent.py→ intentionally empty with documented rationale ✓src/keboola_agent_cli/commands/context.pyAGENT_CONTEXT→agentsubcommand group documented (lines 878–933) ✓CLAUDE.md## All CLI Commands→ 12kbagent agententries present (lines 463–474) ✓plugins/kbagent/agents/keboola-expert.md§ Tool Selection Matrix → newagent <verb>row at line 180 ✓; Rule 6 VERSION GATE →0.42.0+entry at line 111–115 ✓plugins/kbagent/skills/kbagent/references/commands-reference.md→## Agent Tasks (since v0.42.0)section at line 239 ✓plugins/kbagent/skills/kbagent/references/gotchas.md→ two(since v0.42.0)entries confirmed ✓plugins/kbagent/skills/kbagent/references/agent-tasks-cli-workflow.md→ new file, 214 lines ✓plugins/kbagent/skills/kbagent/references/agent-tasks-rest-workflow.md→ renamed fromagent-tasks-workflow.md, zero content change ✓; no staleagent-tasks-workflow.mdlinks found anywhere in plugins ✓SKILL.md→kbagent agent listentry in auto-generated table at line 301 ✓; workflow rows at lines 353–354 ✓make check(run on main repo, which shares the worktree's installed package) →3339 passed, 7 skipped in 65.00s, exit 0 ✓grep -n 'def test_'intests/test_agent_cli.py→ 13 functions; missing:show,run-detail,run-events,test,prompt-improve✓ (finding B-2 confirmed)grep -n 'prompt.improve'intests/test_agent_cli.py→ empty ✓ (finding B-2 / NB-4 confirmed)tests/test_e2e.py::TestE2EAgentTasks→ 3 E2E tests (cron_preview,full_lifecycle,test_command) ✓;prompt_improveabsent ✓ (finding NB-4)tests/test_agent_service.py→ 23 test methods in class-based tests ✓gh pr list --state open→ PR #308 "Semantic Layer UI" onfeat/semantic-layer-uiis open;gh pr view 308 --json filesshows all 7 shared files overlap with PR #310 diff ✓ (finding B-1 confirmed)git log --oneline feat/agent-cli-parity ^main→ 6 commits;bccf4d1and6e24ae4arefeat(semantic-layer-ui)commits also present onfeat/semantic-layer-uibranch ✓server/routers/agents.pyendpoints: 11 routes enumerated; noPOST /agents/prompt/improve(blocking) found; onlyPOST /agents/prompt/improve/stream(SSE) at line 467 ✓ (finding B-3 confirmed)CliAgentRegistry→ proper@dataclassatagent_service.py:59✓ (CONTRIBUTING.md dataclass rule met)- Magic numbers / bare
except:/ rawerror_code=strings /print()in production → none found ✓ - serve_token printed in startup banner (
commands/serve.py:241) → pre-existing behavior, not introduced by this PR ✓ tokens=display inrunshuman output (commands/agent.py:284) → displays token COUNT (integer), not the token value ✓ (no token exposure)- Behavior reproduction: no live Keboola credentials available in this environment;
agent cron-previewandagent create/show/deletelifecycle exercised via the existing E2E tests as documented in the PR — unverified at runtime in this review session
Open questions for the author
- The two
feat(semantic-layer-ui)commits landed in this branch but belong
to the separately-open PR #308. Was the intent to merge PR #308 into this
branch so thatfeat/agent-cli-parityextends the semantic-layer UI work?
If so, should PR #308 be closed in favour of this one? Clarifying the
intended merge order will prevent a three-way conflict onSemanticLayer.tsx,
SemanticLayerDialogs.tsx,http_base.py, and_semantic_layer_internals.py.
…e REST, expand CLI tests Addresses three blocking findings from the kbagent-pr-reviewer pass on PR #310. The duplicate semantic-layer-ui commits (B-1) were dropped by rebasing onto the new origin/main; the remaining two are fixed here. Version bump 0.42.0 -> 0.44.0: - main has shipped 0.42.0 (workspace discoverability fix) and 0.43.0 (semantic-layer UI) since this branch was cut. The agent CLI parity cannot squat on 0.42.0 anymore. Changelog entries split: 0.44.0 carries the new agent CLI + beta release infra commits; 0.42.0 / 0.43.0 entries preserved verbatim from main. B-2: every one of the twelve `kbagent agent` subcommands now has CliRunner coverage in tests/test_agent_cli.py. New classes (9 tests): - TestAgentShow: happy-path + NOT_FOUND - TestAgentRunDetail: happy-path (uses mocked run) + NOT_FOUND - TestAgentRunEvents: cli_command runs have no timeline -> NOT_FOUND - TestAgentTest: success without persistence + MISSING_PARAMETER - TestAgentPromptImprove: happy-path with --no-stream + empty-goal VALIDATION_ERROR 22 CLI tests total (was 13). The changelog entry under 0.44.0 is updated to the new count. B-3: new blocking `POST /agents/prompt/improve` REST endpoint in routers/agents.py (~50 LOC). Mirrors the SSE variant byte-for-byte by consuming `stream_ai_agent_events` server-side and returning the final `done` payload directly (with `data.prompt` cleaned). Closes the 1:1 CLI<->REST parity gap CONTRIBUTING.md requires: the CLI's `agent prompt-improve --no-stream` mode now has a matching backend without forcing scripted callers to open an SSE connection just to read the final frame. Other touch-ups from the rebase: - changelog.py: reordered so new release entries are newest-first (0.44.0 first, then 0.43.0, then 0.42.0). - pyproject.toml + plugin.json + marketplace.json: version 0.44.0 (synced via make version-sync). - keboola-expert.md: tightened the Tool Selection Matrix row + Rule 6 VERSION GATE entry to stay under the 60 KB token budget after origin/main's 0.43.0 additions landed. - SKILL.md auto-regenerated by make skill-gen (decision-table row count).
f9c77f5 to
2098fab
Compare
…e REST, expand CLI tests Addresses three blocking findings from the kbagent-pr-reviewer pass on PR #310. The duplicate semantic-layer-ui commits (B-1) were dropped by rebasing onto the new origin/main; the remaining two are fixed here. Version bump 0.42.0 -> 0.44.0: - main has shipped 0.42.0 (workspace discoverability fix) and 0.43.0 (semantic-layer UI) since this branch was cut. The agent CLI parity cannot squat on 0.42.0 anymore. Changelog entries split: 0.44.0 carries the new agent CLI + beta release infra commits; 0.42.0 / 0.43.0 entries preserved verbatim from main. B-2: every one of the twelve `kbagent agent` subcommands now has CliRunner coverage in tests/test_agent_cli.py. New classes (9 tests): - TestAgentShow: happy-path + NOT_FOUND - TestAgentRunDetail: happy-path (uses mocked run) + NOT_FOUND - TestAgentRunEvents: cli_command runs have no timeline -> NOT_FOUND - TestAgentTest: success without persistence + MISSING_PARAMETER - TestAgentPromptImprove: happy-path with --no-stream + empty-goal VALIDATION_ERROR 22 CLI tests total (was 13). The changelog entry under 0.44.0 is updated to the new count. B-3: new blocking `POST /agents/prompt/improve` REST endpoint in routers/agents.py (~50 LOC). Mirrors the SSE variant byte-for-byte by consuming `stream_ai_agent_events` server-side and returning the final `done` payload directly (with `data.prompt` cleaned). Closes the 1:1 CLI<->REST parity gap CONTRIBUTING.md requires: the CLI's `agent prompt-improve --no-stream` mode now has a matching backend without forcing scripted callers to open an SSE connection just to read the final frame. Other touch-ups from the rebase: - changelog.py: reordered so new release entries are newest-first (0.44.0 first, then 0.43.0, then 0.42.0). - pyproject.toml + plugin.json + marketplace.json: version 0.44.0 (synced via make version-sync). - keboola-expert.md: tightened the Tool Selection Matrix row + Rule 6 VERSION GATE entry to stay under the 60 KB token budget after origin/main's 0.43.0 additions landed. - SKILL.md auto-regenerated by make skill-gen (decision-table row count).
2098fab to
5f06fe0
Compare
…e REST, expand CLI tests Addresses three blocking findings from the kbagent-pr-reviewer pass on PR #310. The duplicate semantic-layer-ui commits (B-1) were dropped by rebasing onto the new origin/main; the remaining two are fixed here. Version bump 0.42.0 -> 0.44.0: - main has shipped 0.42.0 (workspace discoverability fix) and 0.43.0 (semantic-layer UI) since this branch was cut. The agent CLI parity cannot squat on 0.42.0 anymore. Changelog entries split: 0.44.0 carries the new agent CLI + beta release infra commits; 0.42.0 / 0.43.0 entries preserved verbatim from main. B-2: every one of the twelve `kbagent agent` subcommands now has CliRunner coverage in tests/test_agent_cli.py. New classes (9 tests): - TestAgentShow: happy-path + NOT_FOUND - TestAgentRunDetail: happy-path (uses mocked run) + NOT_FOUND - TestAgentRunEvents: cli_command runs have no timeline -> NOT_FOUND - TestAgentTest: success without persistence + MISSING_PARAMETER - TestAgentPromptImprove: happy-path with --no-stream + empty-goal VALIDATION_ERROR 22 CLI tests total (was 13). The changelog entry under 0.44.0 is updated to the new count. B-3: new blocking `POST /agents/prompt/improve` REST endpoint in routers/agents.py (~50 LOC). Mirrors the SSE variant byte-for-byte by consuming `stream_ai_agent_events` server-side and returning the final `done` payload directly (with `data.prompt` cleaned). Closes the 1:1 CLI<->REST parity gap CONTRIBUTING.md requires: the CLI's `agent prompt-improve --no-stream` mode now has a matching backend without forcing scripted callers to open an SSE connection just to read the final frame. Other touch-ups from the rebase: - changelog.py: reordered so new release entries are newest-first (0.44.0 first, then 0.43.0, then 0.42.0). - pyproject.toml + plugin.json + marketplace.json: version 0.44.0 (synced via make version-sync). - keboola-expert.md: tightened the Tool Selection Matrix row + Rule 6 VERSION GATE entry to stay under the 60 KB token budget after origin/main's 0.43.0 additions landed. - SKILL.md auto-regenerated by make skill-gen (decision-table row count).
5f06fe0 to
4ab5a39
Compare
* feat(0.42.0): kbagent update --beta -- opt-in for pre-release channel
PEP 440 + GitHub --prerelease flag = two independent gates that keep
stable users safe from accidentally landing on a beta release.
Default path (unchanged):
- _fetch_kbagent_latest_version() hits /releases/latest -- GitHub
defines this endpoint as "the most recent non-prerelease, non-draft
release", so betas marked --prerelease are invisible to the auto-update
startup hook.
- build_kbagent_upgrade_command() returns the same uv tool install
--upgrade ... cmd as before -- uv defaults to rejecting PEP 440
pre-release version specs.
Beta opt-in (kbagent update --beta or KBAGENT_INCLUDE_PRERELEASE=1):
- Version fetcher switches to /releases (plural), filters drafts,
parses every tag through packaging.version.Version, returns the
highest by PEP 440 ordering. Stable hot-fixes to an older release
line still win over a more-recently-tagged beta on a newer line.
- Resolver opt-in: --prerelease=allow (uv) / --pre (pip) inserted into
the install command so the resolver accepts 0.43.0b1 even though
it's a pre-release.
UX choices:
- Ad-hoc only -- no release_channel: beta persistent config setting.
Each opt-in is an active choice (--beta or env var per shell). This
prevents the "I once typed --beta six months ago and forgot" foot-gun.
- env var KBAGENT_INCLUDE_PRERELEASE=1 supports the CI-smoke-test use
case (don't want to re-type --beta in every cron job step).
- Startup auto-update hook untouched -- it still never auto-installs
a beta. Only the explicit kbagent update --beta does.
Tests: 10 new in test_version_service.py:
- TestFetchKbagentLatestVersion: default uses /releases/latest endpoint;
prerelease uses /releases and returns highest by PEP 440 ordering;
prerelease skips drafts; falls back to stable when no betas; ignores
invalid tags; HTTP failure returns None.
- TestBuildKbagentUpgradeCommand: uv flag insertion (with and without
[server] extras), pip --pre fallback.
Docs:
- CONTRIBUTING.md: new "Releasing a beta (pre-release) version" section
documenting the PEP 440 + gh release create --prerelease workflow,
the two-gate model, and the two opt-in paths.
- CLAUDE.md `## All CLI Commands`: `version [--beta]` and `update [--beta]`
with a one-line note about the env var override.
- context.py: per-command help refreshed.
- commands-reference.md: same.
- changelog.py: 0.42.0 entry (mixed with the agent CLI feature in the
same release).
* docs: document beta release workflow in CLAUDE.md + CONTRIBUTING.md
CLAUDE.md `## Versioning` section gains a `### Beta / pre-release
versions (since 0.42.0)` subsection covering:
- Why PEP 440 (`0.43.0b1`) instead of SemVer (`-beta.1`)
- The two independent gates (pre-release suffix + GitHub --prerelease)
- Author workflow (4-step recipe from version bump to gh release)
- User opt-in paths (--beta flag, env var, no persistent setting)
- Cross-reference to CONTRIBUTING.md for the full author checklist
CONTRIBUTING.md `## Releasing a new version` intro gains a blockquote
signposting the beta workflow so authors discover the option without
having to scroll to the end of the section.
* fix(0.43.3): tag-pinned install URL for kbagent update --beta
The v0.42.0 implementation of --beta opt-in had a latent bug that
only fires when a beta tag lives on a feature branch (not main):
build_kbagent_upgrade_command(prerelease=True) generated
uv tool install --force --with keboola-agent-cli[server]
--prerelease=allow git+https://github.com/padak/keboola_agent_cli
uv resolves the git+ URL by checking out the DEFAULT BRANCH (main)
and reading pyproject.toml at HEAD. So even though _fetch_kbagent_
latest_prerelease() advertised "0.44.0b1", uv would silently install
whatever main carried at HEAD (e.g. 0.43.2) -- because the tag was
never consulted. The user sees "kbagent update --beta -> 0.44.0b1"
in the version banner and ends up with 0.43.2.
The fix tag-pins the install URL when (and only when) opting into
prerelease:
build_kbagent_upgrade_command(prerelease=True,
target_version="0.44.0b1")
now appends @v0.44.0b1 to the git+ URL, so uv installs the exact
commit the tag points to. _update_kbagent forwards the value it
already had from _fetch_kbagent_latest_version().
Stable upgrades intentionally pass target_version=None: main IS the
stable channel, so tag-pinning would just add a needless HTTP round-
trip without changing the resolved version. The default behaviour
(no --beta, no tag suffix) is byte-for-byte unchanged from 0.43.2.
Tests:
- test_uv_prerelease_with_target_version_appends_tag: pre-release
path emits @v<version> in install spec
- test_uv_stable_with_target_version_ignores_tag: target_version is
silently ignored when prerelease=False (no regression for stable)
Docs:
- CLAUDE.md "Beta / pre-release versions" section reworded: third
gate (tag-pinned install URL) added to the two-gate model, author
workflow renamed to "Releasing a beta from a feature branch"
reflecting the canonical flow (PR head, not main)
- changelog: 0.43.3 entry describes the bug + fix + new gate.
This release-pattern was discovered while preparing PR #310 for
beta release: the agent-cli-parity feature lives on a long-running
PR branch, main is on 0.43.2, and we want kbagent update --beta to
land users on the actual beta tag rather than silently fall back
to main.
* fix(pr-317 review): NB-1/2/3 silent-drift entries for --beta opt-in
Three non-blocking findings from kbagent-pr-reviewer review of PR #317:
NB-1 (keboola-expert.md VERSION GATE):
Rule 6 listed every prior version-gated capability but had no entry
for `kbagent update --beta` / `version --beta`. An AI agent running
on a 0.41.x install asked to "install the latest beta" would have
no warning that --beta requires 0.43.3+. Added compact entry
`\`kbagent update --beta\` = 0.43.3+` to the gate list.
Trimmed two neighbouring entries (#304, #312) by removing redundant
parenthesis spaces to stay under the 60000-byte prompt budget --
the system prompt was already at 99.8% capacity before this fix.
NB-2 (get_versions() upgrade_command):
`kbagent --json version --beta` correctly reported the prerelease
in `kbagent.latest_version` but the `kbagent.upgrade_command` field
was a static `uv tool install --upgrade git+...` string with no
`--prerelease=allow` and no `@v<version>` tag-pin. Downstream
consumers reading the JSON would copy this command, run it, and
silently land on the stable channel even though the field said
"0.44.0b1 is available". get_versions() now calls
`build_kbagent_upgrade_command(prerelease=include_prerelease,
target_version=...)` and joins the result -- the rendered command
now matches what self_update() actually runs.
NB-3 (commands/context.py AGENT_CONTEXT env-var block):
KBAGENT_INCLUDE_PRERELEASE was documented inline next to the
`version`/`update` commands but missing from the dedicated
"Environment Variables" listing at §7. AI agents that scan that
block to discover all overrides would miss it. Added entry next to
KBAGENT_MCP_TRANSPORT with the same shape as the inline help.
Test plan:
- `make check` -> 3393 passed (was 3393 + 1 budget overflow before
trim; now under 60000-byte ceiling at exactly 60000 bytes after
the two-character trim on neighbouring entries)
- get_versions(include_prerelease=False) zero behaviour change ->
upgrade_command identical to prior release for stable callers.
…e REST, expand CLI tests Addresses three blocking findings from the kbagent-pr-reviewer pass on PR #310. The duplicate semantic-layer-ui commits (B-1) were dropped by rebasing onto the new origin/main; the remaining two are fixed here. Version bump 0.42.0 -> 0.44.0: - main has shipped 0.42.0 (workspace discoverability fix) and 0.43.0 (semantic-layer UI) since this branch was cut. The agent CLI parity cannot squat on 0.42.0 anymore. Changelog entries split: 0.44.0 carries the new agent CLI + beta release infra commits; 0.42.0 / 0.43.0 entries preserved verbatim from main. B-2: every one of the twelve `kbagent agent` subcommands now has CliRunner coverage in tests/test_agent_cli.py. New classes (9 tests): - TestAgentShow: happy-path + NOT_FOUND - TestAgentRunDetail: happy-path (uses mocked run) + NOT_FOUND - TestAgentRunEvents: cli_command runs have no timeline -> NOT_FOUND - TestAgentTest: success without persistence + MISSING_PARAMETER - TestAgentPromptImprove: happy-path with --no-stream + empty-goal VALIDATION_ERROR 22 CLI tests total (was 13). The changelog entry under 0.44.0 is updated to the new count. B-3: new blocking `POST /agents/prompt/improve` REST endpoint in routers/agents.py (~50 LOC). Mirrors the SSE variant byte-for-byte by consuming `stream_ai_agent_events` server-side and returning the final `done` payload directly (with `data.prompt` cleaned). Closes the 1:1 CLI<->REST parity gap CONTRIBUTING.md requires: the CLI's `agent prompt-improve --no-stream` mode now has a matching backend without forcing scripted callers to open an SSE connection just to read the final frame. Other touch-ups from the rebase: - changelog.py: reordered so new release entries are newest-first (0.44.0 first, then 0.43.0, then 0.42.0). - pyproject.toml + plugin.json + marketplace.json: version 0.44.0 (synced via make version-sync). - keboola-expert.md: tightened the Tool Selection Matrix row + Rule 6 VERSION GATE entry to stay under the 60 KB token budget after origin/main's 0.43.0 additions landed. - SKILL.md auto-regenerated by make skill-gen (decision-table row count).
4ab5a39 to
d828e83
Compare
Addresses all six findings from the kbagent-pr-reviewer subagent's review of PR keboola#321: [B-1] changelog gap (make changelog-check failed) Added 0.44.0b1 entry to src/keboola_agent_cli/changelog.py mirroring the published GitHub pre-release notes (kbagent agent <verb> CLI parity, opt-in via --beta). The tag is padak's own published beta from open PR keboola#310; this entry is a stop-gap that keboola#310's eventual merge will normalize. [B-2] kbagent serve REST mirror silently dropped --mode debug server/routers/jobs.py JobRun Pydantic model now carries mode: str = DEFAULT_JOB_MODE and the run() endpoint passes mode=body.mode through to JobService.run_job. Two new router tests in test_server_smoke.py (default lands as mode='run', mode='debug' reaches the service) lock the 1:1 CLI <-> REST parity contract. [NB-2] no E2E coverage for --mode debug New TestE2EJobRunMode class in tests/test_e2e.py submits a real --mode debug job against a live Keboola project and asserts the Queue API echoes mode='debug' on the create response -- the wire-level proof. Best-effort kill_job avoids waste compute. Verified locally against project 4214. [NIT-1] inline import json in two new test methods Dropped; module-level json import at tests/test_client.py:4 already covers both new TestCreateJob tests. [NIT-2] mode validation ordering Kept the mode guard grouped with poll_strategy (both are O(1) frozenset enum-membership checks failing in the same INVALID_ARGUMENT shape) and added a comment explaining the ordering is intentional. [NB-1] keboola-expert.md "Debug a failed job" row not updated Deferred per the reviewer's own note (60000-byte agent prompt budget with 5 bytes headroom). The --mode debug flag remains discoverable to the agent via three other on-demand-loaded surfaces: AGENT_CONTEXT (kbagent context bootstrap), commands-reference.md, and the new (since v0.43.6) entry in gotchas.md. make check now passes all 7 gates (was failing at changelog-check). 3411 unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per padak's review: the v0.44.0b1 GitHub pre-release is his experimental beta from open PR keboola#310, not a release the 0.43.6 changelog should be papering over. Dropping the stop-gap entry I added to satisfy `make changelog-check` -- padak will handle his beta on his end (either by merging keboola#310 with the proper changelog entry, or by managing the release tag separately). CI's `make changelog-check` will fail until that lands, but that's a pre-existing condition this PR shouldn't paper over. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(0.43.6): kbagent job run --mode run|debug exposes Queue API debug mode The Queue API has always accepted a `mode` body field on job creation (`run` writes to mapped output tables; `debug` redirects the worker's output to a Storage File tagged `debug-<jobId>` instead of into the destination buckets). `KeboolaClient.create_job` already accepted a `mode` kwarg, but neither `JobService.run_job` nor `commands/job.py` threaded it through, so every job kbagent submitted was hard-coded to `run`. This wires the parameter through all three layers and adds `--mode run|debug` on the CLI (default `run`, unchanged wire shape). Validation lives at both the service boundary (`VALID_JOB_MODES`, `KeboolaApiError` INVALID_ARGUMENT) and the Click choice gate, so a typo exits 2 before any wire call rather than surfacing as an opaque 422. Motivating use case: AI agents that develop and test Keboola components can now run a production-shaped config in debug mode, harvest the debug-tagged output file via `storage file-download --tag debug-<jobId>`, and use those bytes as ground-truth fixtures for new VCR recordings and component test cases -- all without writing through to live downstream tables. The human-mode banner appends a bold-yellow `mode=debug` chip when the flag is non-default so operators see at a glance that a run is diagnostic. The hint surface (`--hint client` / `--hint service`) emits the `mode="..."` kwarg on both the `create_job` and `JobService.run_job` rendered calls. Test coverage: 3 service-layer tests (default lands as run, debug forwarded, unknown mode rejected before reaching the wire), 3 CLI tests (default, --mode debug forwarded, --mode dry-run rejected by Click), 2 client-layer tests (default mode in the POST /jobs body, debug reaches the body verbatim). 4 existing `create_job.assert_called_once_with` service assertions updated to include `mode="run"`. Plugin sync surfaces updated (silent-drift risks per convention #17): CLAUDE.md command list, `commands/context.py` AGENT_CONTEXT, `commands-reference.md`, new `(since v0.43.6)` entry in `gotchas.md`, and the "Debug a failed job" row in `keboola-expert.md`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert: drop keboola-expert.md edit (60000-byte prompt budget) The Debug-a-failed-job row update pushed the agent prompt to 60236 bytes, 236 over the hard budget enforced by tests/test_agent_prompt.py::test_agent_prompt_under_token_budget. The --mode debug flag stays documented in three other surfaces the agent skill loads on demand: AGENT_CONTEXT (loaded by every "kbagent context" call -- the skill's first-step bootstrap), commands-reference.md, and a fresh (since v0.43.6) section in gotchas.md. Drift risk is acceptably low; the keboola-expert table row can be re-introduced when other content gets trimmed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(review): address /kbagent:review findings on #321 Addresses all six findings from the kbagent-pr-reviewer subagent's review of PR #321: [B-1] changelog gap (make changelog-check failed) Added 0.44.0b1 entry to src/keboola_agent_cli/changelog.py mirroring the published GitHub pre-release notes (kbagent agent <verb> CLI parity, opt-in via --beta). The tag is padak's own published beta from open PR #310; this entry is a stop-gap that #310's eventual merge will normalize. [B-2] kbagent serve REST mirror silently dropped --mode debug server/routers/jobs.py JobRun Pydantic model now carries mode: str = DEFAULT_JOB_MODE and the run() endpoint passes mode=body.mode through to JobService.run_job. Two new router tests in test_server_smoke.py (default lands as mode='run', mode='debug' reaches the service) lock the 1:1 CLI <-> REST parity contract. [NB-2] no E2E coverage for --mode debug New TestE2EJobRunMode class in tests/test_e2e.py submits a real --mode debug job against a live Keboola project and asserts the Queue API echoes mode='debug' on the create response -- the wire-level proof. Best-effort kill_job avoids waste compute. Verified locally against project 4214. [NIT-1] inline import json in two new test methods Dropped; module-level json import at tests/test_client.py:4 already covers both new TestCreateJob tests. [NIT-2] mode validation ordering Kept the mode guard grouped with poll_strategy (both are O(1) frozenset enum-membership checks failing in the same INVALID_ARGUMENT shape) and added a comment explaining the ordering is intentional. [NB-1] keboola-expert.md "Debug a failed job" row not updated Deferred per the reviewer's own note (60000-byte agent prompt budget with 5 bytes headroom). The --mode debug flag remains discoverable to the agent via three other on-demand-loaded surfaces: AGENT_CONTEXT (kbagent context bootstrap), commands-reference.md, and the new (since v0.43.6) entry in gotchas.md. make check now passes all 7 gates (was failing at changelog-check). 3411 unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert: drop 0.44.0b1 changelog entry (padak owns the beta) Per padak's review: the v0.44.0b1 GitHub pre-release is his experimental beta from open PR #310, not a release the 0.43.6 changelog should be papering over. Dropping the stop-gap entry I added to satisfy `make changelog-check` -- padak will handle his beta on his end (either by merging #310 with the proper changelog entry, or by managing the release tag separately). CI's `make changelog-check` will fail until that lands, but that's a pre-existing condition this PR shouldn't paper over. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
make changelog-check (scripts/generate_changelog.py --check) demanded a CHANGELOG entry for every GitHub release tag, including pre-releases. An in-flight beta tag (v0.44.0b1, from the unmerged 'kbagent agent' PR #310) therefore made local 'make check' red on every branch -- the beta's changelog entry lives on its own feature branch until that PR merges, per the CONTRIBUTING.md beta workflow. Skip pre-release tags in the coverage check (gh release list now also fetches isPrerelease), mirroring the auto-update path which only sees stable releases via /releases/latest. The filtering is extracted into a pure, I/O-free audit_changelog_coverage(tags, changelog) helper covered by tests/test_changelog_check.py (6 tests). Surfaced by the PR #322 review. changelog-check is local-only (not in CI) so this never turned CI red, but it blocked a clean local 'make check'.
* fix(0.43.7): repair Windows wheel build hook (closes #320) Windows installation via 'uv tool install git+...' was completely broken by two independent bugs in hatch_build.py, with no working install path. Bug 1 (npm present): npm is npm.cmd on Windows; a bare subprocess.check_call(["npm", ...]) raises FileNotFoundError (an OSError subclass, not CalledProcessError), so the build crashed with WinError 2. Fix: pass the resolved shutil.which("npm") path (which is npm.cmd on Windows and runs via the system shell even with shell=False) and widen the except to (CalledProcessError, OSError) so a spawn failure degrades to a UI-less wheel instead of aborting the build. Bug 2 (npm absent): an early return left _ui_dist/ missing, but the unconditional force-include in pyproject.toml requires it to exist, so hatchling failed with 'Forced include not found'. Fix: every code path now guarantees _ui_dist/ exists via _ensure_target() (an empty dir is enough; the runtime UI detector keys on index.html). Build logic is extracted into a pure, hatchling-free _bundle_ui() so it is unit-testable in a plain dev venv. New KBAGENT_SKIP_UI_BUILD=1 env var ships a deliberate CLI-only wheel. Verified without a Windows machine: - tests/test_build_hook.py (20 tests) reproduces both bugs via mocks on any OS and asserts _ui_dist/ always exists; full suite 3428 passed. - New windows-latest CI job runs a real uv build on a free GitHub runner and asserts (scripts/check_wheel_ui.py) that a normal build bundles _ui_dist/index.html while a KBAGENT_SKIP_UI_BUILD=1 build does not. No CLI command surface changed (only a build-time env var). * fix: changelog-check skips pre-release tags make changelog-check (scripts/generate_changelog.py --check) demanded a CHANGELOG entry for every GitHub release tag, including pre-releases. An in-flight beta tag (v0.44.0b1, from the unmerged 'kbagent agent' PR #310) therefore made local 'make check' red on every branch -- the beta's changelog entry lives on its own feature branch until that PR merges, per the CONTRIBUTING.md beta workflow. Skip pre-release tags in the coverage check (gh release list now also fetches isPrerelease), mirroring the auto-update path which only sees stable releases via /releases/latest. The filtering is extracted into a pure, I/O-free audit_changelog_coverage(tags, changelog) helper covered by tests/test_changelog_check.py (6 tests). Surfaced by the PR #322 review. changelog-check is local-only (not in CI) so this never turned CI red, but it blocked a clean local 'make check'.
…face Twelve subcommands matching the REST endpoints byte-for-byte: list, show, create, update, delete, run [--stream] [--runtime-prompt], runs, run-detail, run-events, test [--stream], cron-preview, prompt-improve [--stream/--no-stream]. Pure-local on <config_dir>/agents.json -- CRUD + ad-hoc run work offline. The cron loop that fires scheduled tasks still requires kbagent serve running, but the on-disk format is identical, so a CLI-created task fires on its cron as soon as the server boots. Action flavours mirror REST exactly (ai_agent / cli_command / mcp_tool); --from-file PATH|@path|- accepts the full {type, params} JSON envelope. Convenience flags cover the common single-action case: --type ai_agent --cli claude|codex|gemini --prompt P [--extra-arg] --type cli_command --argv ARG [--argv ARG ...] --type mcp_tool --tool T [--mcp-project ALIAS] [--mcp-branch ID] [--input JSON|@file|-] Streaming variants render events line-by-line in human mode, NDJSON in --json mode -- one packet per claude/codex/gemini stdout line, plus init/done envelopes. Live cancellation via Ctrl+C kills the spawned subprocess via the async-generator finally block. Trigger chaining via --trigger-task-id ID --trigger-on success|error|always with the same validation the REST router applies (no self-loops, target must exist). validate_trigger + merge_runtime_input extracted from routers/agents.py into server/agents_store.py so REST + CLI share the exact same boundary behaviour. croniter moved from [server] extras to core dependencies (cron-preview validation needs to work outside serve). server/__init__.py split into a PEP 562 lazy shim + new server/app.py so importing from keboola_agent_cli.server.agents_store import AgentStore no longer drags FastAPI/uvicorn into the CLI path -- the agent CLI works on plain installs without [server] extras. Permission registry adds 12 entries (read for inspection, write for mutation + run + test + prompt-improve, destructive for delete). Hint definitions ship an intentionally-empty agent.py with rationale (agent CRUD is pure-local, no HTTP client to hint at). Tests: - tests/test_agent_service.py: 23 unit tests against AgentService with a real AgentStore on tmp dirs. CRUD round-trip, cron validation, trigger validation incl. self-loop + missing target, runtime_input merge, run mocking, stream test action, run history NOT_FOUND mapping. - tests/test_agent_cli.py: 13 CLI tests via CliRunner against a real tmp config dir, both human and --json modes. Covers all 12 subcommands and the runtime-prompt merge path. - tests/test_e2e.py::TestE2EAgentTasks: 3 E2E tests (cron-preview, full create-show-update-run-runs-delete lifecycle with cli_command action, ad-hoc test). Plugin docs sync (every silent-drift surface): - New skill reference: agent-tasks-cli-workflow.md (CLI-first walkthrough). - Renamed: agent-tasks-workflow.md -> agent-tasks-rest-workflow.md (kept for AI-agent subprocess REST callbacks). - CLAUDE.md ## All CLI Commands: full agent cheat sheet. - commands/context.py AGENT_CONTEXT: per-subcommand documentation. - keboola-expert.md: Rule 6 version gate entry + Tool Selection Matrix row for Agent Tasks. - SKILL.md: auto-regenerated table + manual workflow rows. - commands-reference.md: new "Agent Tasks (since v0.42.0)" section. - gotchas.md: two new (since v0.42.0) entries (offline CRUD vs cron firing requires serve; shared validate_trigger / merge_runtime_input helpers). Closes the gap between the React UI sidebar "Agent Tasks" (which had this surface since v0.40.0) and the CLI users who had to fall back to kbagent http <verb> /agents... from inside scheduled subprocesses.
…e REST, expand CLI tests Addresses three blocking findings from the kbagent-pr-reviewer pass on PR #310. The duplicate semantic-layer-ui commits (B-1) were dropped by rebasing onto the new origin/main; the remaining two are fixed here. Version bump 0.42.0 -> 0.44.0: - main has shipped 0.42.0 (workspace discoverability fix) and 0.43.0 (semantic-layer UI) since this branch was cut. The agent CLI parity cannot squat on 0.42.0 anymore. Changelog entries split: 0.44.0 carries the new agent CLI + beta release infra commits; 0.42.0 / 0.43.0 entries preserved verbatim from main. B-2: every one of the twelve `kbagent agent` subcommands now has CliRunner coverage in tests/test_agent_cli.py. New classes (9 tests): - TestAgentShow: happy-path + NOT_FOUND - TestAgentRunDetail: happy-path (uses mocked run) + NOT_FOUND - TestAgentRunEvents: cli_command runs have no timeline -> NOT_FOUND - TestAgentTest: success without persistence + MISSING_PARAMETER - TestAgentPromptImprove: happy-path with --no-stream + empty-goal VALIDATION_ERROR 22 CLI tests total (was 13). The changelog entry under 0.44.0 is updated to the new count. B-3: new blocking `POST /agents/prompt/improve` REST endpoint in routers/agents.py (~50 LOC). Mirrors the SSE variant byte-for-byte by consuming `stream_ai_agent_events` server-side and returning the final `done` payload directly (with `data.prompt` cleaned). Closes the 1:1 CLI<->REST parity gap CONTRIBUTING.md requires: the CLI's `agent prompt-improve --no-stream` mode now has a matching backend without forcing scripted callers to open an SSE connection just to read the final frame. Other touch-ups from the rebase: - changelog.py: reordered so new release entries are newest-first (0.44.0 first, then 0.43.0, then 0.42.0). - pyproject.toml + plugin.json + marketplace.json: version 0.44.0 (synced via make version-sync). - keboola-expert.md: tightened the Tool Selection Matrix row + Rule 6 VERSION GATE entry to stay under the 60 KB token budget after origin/main's 0.43.0 additions landed. - SKILL.md auto-regenerated by make skill-gen (decision-table row count).
d828e83 to
90be77d
Compare
…isibility Follow-up to PR #310 review (4 self-identified improvements): 1. version-sync now patches uv.lock's self-version pin (was only plugin.json + marketplace.json); version-check guards all three. 2. agent subcommands accept TASK_ID/RUN_ID via --id/--task-id/--run-id flags as an alias to the positional argument, matching the rest of the CLI (--job-id, --config-id, ...). Positional form still works. 3. post-edit hook surfaces ty warnings under a clear header (still warning-only; flipping to blocking is tracked separately). 4. CONTRIBUTING.md documents the beta re-tag (rebase -> bN+1) recipe. Tests: 3548 passed. New coverage: 5 uv.lock sync tests, 7 agent --id/--task-id/--run-id alias tests (incl. conflict + missing-id paths).
padak
left a comment
There was a problem hiding this comment.
Review of #310 — feat(0.42.0): kbagent agent -- CLI parity for /agents REST surface
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 PR adds twelve kbagent agent <verb> subcommands (CRUD + run + history + utilities) that give the CLI parity with the /agents REST surface already exposed by kbagent serve since v0.40.0. The architecture is clean: boundary helpers (validate_trigger, merge_runtime_input) extracted from the REST router into agents_store.py are shared byte-for-byte by both the CLI service and the HTTP router; the minimal CliAgentRegistry dataclass avoids pulling the full FastAPI ServiceRegistry for single CLI runs; and the PEP 562 lazy-import shim in server/__init__.py prevents FastAPI from loading on plain (non-server-extras) installs. make check passes (3548 tests). The _resolve_id helper and all edge cases (positional-only, flag-only, conflict, missing) are correctly tested.
One BLOCKING issue must be resolved before merge: the changelog.py entry for the agent CLI feature sits under the key "0.42.0", which is already a shipped stable release (the workspace/QS release). When this branch is eventually tagged and released as stable 0.44.0, make changelog-check will fail because no "0.44.0" key exists in changelog.py. Two docs surfaces also tag the feature as (since v0.42.0) when the correct version is v0.44.0 — the keboola-expert.md already says 0.44.0+ correctly, but commands-reference.md and gotchas.md diverge.
Verdict: REQUEST CHANGES (one BLOCKING finding; the rest are non-blocking or cosmetic).
Verdict
- Verdict: REQUEST CHANGES
- Blocking findings: 1
- Non-blocking findings: 4
- Nits: 2
Blocking findings
[B-1] src/keboola_agent_cli/changelog.py:59 — agent CLI feature entry under wrong version key "0.42.0"
The agent CLI feature is documented under the "0.42.0" key in changelog.py, but v0.42.0 is an already-shipped stable GitHub release (the workspace/QS discoverability release). The PR's pyproject.toml correctly reads version = "0.44.0b2" and the keboola-expert.md version gate correctly says 0.44.0+. When this PR is eventually merged and tagged as stable 0.44.0, make changelog-check (which fetches stable GitHub tags and requires a matching dict key) will fail with a missing "0.44.0" entry -- blocking every subsequent make check run on main.
Fix: add a "0.44.0" key to changelog.py and move the agent CLI entry under it. Remove the agent CLI bullet from the "0.42.0" key (the workspace/QS text that belongs to the existing shipped v0.42.0 release can stay). Also update the PR title from feat(0.42.0): to feat(0.44.0):.
Non-blocking findings
[NB-1] plugins/kbagent/skills/kbagent/references/commands-reference.md:240 and references/gotchas.md:1998,2023 — (since v0.42.0) version tags contradict keboola-expert.md
commands-reference.md says ## Agent Tasks (since v0.42.0) and gotchas.md carries two entries tagged (since v0.42.0). But keboola-expert.md correctly says kbagent agent <verb> ... = 0.44.0+. The mismatch means AI agents that read commands-reference.md or gotchas.md will recommend agent commands to users on 0.42.x where they do not exist, producing "command not found" failures. Per CONTRIBUTING.md > "Documentation changes": version tags in gotchas.md are non-optional.
Fix: change both (since v0.42.0) tags to (since v0.44.0) in commands-reference.md and gotchas.md.
[NB-2] src/keboola_agent_cli/commands/agent.py — no should_hint guard on any of the 12 new commands
Every CLI command that does not qualify as infrastructure-only is required by CONTRIBUTING.md to have if should_hint(ctx): emit_hint(...) before the service call so that kbagent --hint client agent show <id> short-circuits cleanly. The agent commands intentionally have an empty hint definition (correct: there is no Keboola HTTP API to mimic), but without the guard, passing --hint silently falls through and executes the real service call instead of exiting with a "No hint available" message.
The CONTRIBUTING.md exception clause covers "commands that have no Keboola HTTP API client to mimic" — which does apply here, and the hints/definitions/agent.py rationale comment correctly explains this. However the correct pattern when a command is excluded from hints is to add a minimal if should_hint(ctx): formatter.error(...); raise typer.Exit(2) guard rather than silently ignoring the flag. The http.py commands set the precedent for this pattern.
Fix: add a should_hint short-circuit at the top of each agent command (or in the _agent_permission_check callback) that prints a clear "agent commands are local-only; no --hint code available" message and exits 0 instead of proceeding with the service call.
[NB-3] CLAUDE.md:469-476 and src/keboola_agent_cli/commands/context.py:919,937,944,947,954,957,960 — signatures show TASK_ID / RUN_ID as required positionals, obscuring the new --id/--run-id flag form
Both the CLAUDE.md "All CLI Commands" section and the AGENT_CONTEXT string document the signatures as:
kbagent agent show TASK_ID
kbagent agent run TASK_ID [--stream]
kbagent agent run-detail TASK_ID RUN_ID
This looks like a required positional argument. Since b5dd132, the positional is optional and --id/--task-id/--run-id are preferred (to match the rest of the CLI). Users reading context or CLAUDE.md will not discover the flag form. The context.py intro paragraph does mention both forms briefly (line 912), but the individual command entries do not show the bracket form [TASK_ID] or the [--id TASK_ID] alternative.
Fix: update the 7 affected lines in both files to show the optional positional bracket form: kbagent agent show [TASK_ID] [--id TASK_ID] (or the terse TASK_ID|--id TASK_ID convention used elsewhere in the codebase).
[NB-4] tests/test_agent_cli.py — no test for runs or run commands accepting the --id / --task-id flag alias
TestIdFlagAliases covers show (positional, --id, --task-id, delete --id), the conflict path, the missing-ID path, and run-detail --id + --run-id. It does not test run --id or runs --id. Both commands use _resolve_id, so the happy-path flag form is untested for them. A future refactor could accidentally drop the task_id_opt parameter from agent_run or agent_runs without a failing test.
Fix: add test_run_accepts_id_flag and test_runs_accepts_id_flag to TestIdFlagAliases.
Nits
-
[NIT-1]src/keboola_agent_cli/commands/agent.py:568-572and:622-627— theagent_createandagent_updatehuman formatters use multi-line lambda tuples (lambda c, d: (c.print(...), _render_task_detail(c, d))). CONTRIBUTING.md "Code Quality Patterns > Named functions over throwaway lambdas" says: "Anything that... doing branching, or carrying domain meaning gets a nameddef." These lambdas carry domain meaning (print a creation/update confirmation then the detail panel) and are slightly opaque. Consider extracting a_render_created_task/_render_updated_tasknamed function, matching the pattern used forrun-detail(which already defines a named_renderinner function at line 779). -
[NIT-2]plugins/kbagent/skills/kbagent/references/commands-reference.md:240— the section heading## Agent Tasks (since v0.42.0)once the version tag is corrected tov0.44.0will also need thesincetag changed fromv0.42.0tov0.44.0. If the separate ID-forms gotcha (which sayssince v0.44.0ingotchas.md) remains a standalone entry, consider adding a brief cross-reference in thecommands-reference.mdagent section so readers discover both the basic form and the flag-alias form together.
Verification log
gh pr view 310 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ 31 files, +3820/-813, state OPEN ✓git rev-parse --abbrev-ref HEAD→feat/agent-cli-parity✓ (matches PR branch)Read CONTRIBUTING.md→ loaded Plugin synchronization map, Checklist, Code Quality Patterns ✓Read CLAUDE.md→ loaded convention #17, All CLI Commands section ✓Read plugins/kbagent/agents/keboola-expert.md§1-§3 → loaded VERSION GATE, Tool Selection Matrix, Inline Gotchas ✓make check→ 3548 passed, 7 skipped, exit 0 ✓- Layer violation scan (
grep -E '^\+(from typer|import typer...)' diff | grep services/) → empty ✓ - Layer violation scan (
grep -E '^\+(from httpx|import httpx...)' diff | grep commands/) → empty ✓ grep -E '^\+.*error_code\s*=\s*"[A-Z_]+"' diff→ empty (all useErrorCodeenum) ✓grep -E '^\+\s*except\s*:' diff→ empty (no bare except) ✓grep -E '^\+\s*print\(' diff | grep src/→ empty ✓grep 'agent.*run.*detail\|run_detail\|_resolve_id' commands/agent.py→ verified all 7 subcommands wire_resolve_id; logic verified manually for positional-only, flag-only, conflict, missing-ID, and same-value-both-ways cases ✓grep '^version' pyproject.toml→version = "0.44.0b2"(beta)gh release view v0.42.0 --json tagName,body→ confirmed v0.42.0 is the workspace/QS release (no agent CLI); the"0.42.0"key inchangelog.pyincorrectly contains the agent CLI entry — see B-1make version-check→version is in sync (plugin.json, marketplace.json, uv.lock)✓make changelog-check→All 48 stable releases have changelog entries. (2 pre-release(s) skipped)✓ (passes today because0.44.0is not yet a stable tag; will fail after final release tag — see B-1)- Plugin synchronization map check:
OPERATION_REGISTRY— 12 new entries present ✓;commands/context.py AGENT_CONTEXT— present ✓;CLAUDE.md ## All CLI Commands— present ✓;keboola-expert.mdRule 6 VERSION GATE0.44.0+✓, Tool Selection Matrix row ✓;commands-reference.md— present but wrong version tag (NB-1);gotchas.md— two entries but wrong version tags (NB-1);agent-tasks-cli-workflow.md— new file present ✓;hints/definitions/agent.py— intentionally empty with rationale comment ✓ async def stream_run(...)inagent_service.pyhasyield→ confirmed async generator; callingservice.stream_run(...)returns the async generator directly (noawaitneeded) →_stream_to_stdoutusage at line 710 is correct ✓from keboola_agent_cli.server import create_appin test files → confirmed PEP 562 shim inserver/__init__.pyforwards the symbol lazily ✓;make checkpassing with 6 test files using this import confirms backward compat ✓- Behavior reproduction (local-only, no network required):
kbagent agent create/list/show/delete— not run directly; verified via 3548-test suite including 29 CLI tests and 23 service tests ✓; E2E tests inTestE2EAgentTasksrequire live API — not run (acceptable per convention) - PR title says
feat(0.42.0):— stale; actual target version is0.44.0per pyproject.toml. Not a blocking issue on its own but related to B-1.
Open questions for the author
commands/agent.py:419—_stream_to_stdoutannotates itsagenparameter asAsyncIterator[dict[str, Any]]. In practicestream_runandstream_test_actionare async-generator functions (theyyield), not coroutines that return an iterator. The type annotation is technically accurate (AsyncGeneratoris a subtype ofAsyncIterator), but callingservice.stream_run(task_id, ...)returns the async generator synchronously withoutawait— is this the intended invocation pattern confirmed through testing, or wasasyncio.run(service.stream_run(...))ever considered? (Tests passing confirms it works; this is a clarification question only.)
…ts, named renderers) - NB-1: commands-reference.md + gotchas.md (2x) version tags 0.42.0 -> 0.44.0 (feature actually ships in 0.44.0; keboola-expert.md already correct). - NB-4: add run/runs --id flag alias tests to TestAgentIdAlias. - NIT-1: extract _render_created_task / _render_updated_task named renderers from multi-line lambda tuples in agent_create / agent_update. NB-2 (should_hint guard) verified a non-issue: --hint on agent commands already prints 'No --hint available' + exit 0. NB-3 covered by the existing alias note in CLAUDE.md / context.py. B-1 (0.42.0 changelog dup + missing 0.44.0 key) is handled in the stable-version consolidation commit.
…44.0 - Consolidate 0.44.0b1 + 0.44.0b2 changelog entries into a single stable 0.44.0 entry (agent CLI surface + --id/--task-id/--run-id aliases + version-sync uv.lock hardening). - Drop the duplicate agent CLI bullet from the already-shipped 0.42.0 release key (0.42.0 was the workspace/QS release; the agent CLI ships in 0.44.0). Fixes PR #310 review finding B-1 -- changelog-check will now find a 0.44.0 key when the stable tag is cut. - Bump pyproject 0.44.0b2 -> 0.44.0; version-sync propagates to plugin.json / marketplace.json / uv.lock.
- Annotate params: dict[str, Any] in _action_from_flags (cli_command + mcp_tool branches) -- fixes 3 invalid-assignment ty errors. - Replace 3 multi-line lambda renderers (run / test / prompt-improve) with named functions returning None. The lambdas returned a tuple, which ty flagged as an invalid OutputFormatter.output argument -- NOT a codebase-wide pattern (config/job/storage/flow have 0 such errors). - Switch two # type: ignore[arg-type] to ty-native # ty: ignore[invalid-argument-type] (Astral ty does not honor the mypy arg-type code) with one-line rationale comments per CONTRIBUTING.md. New code now passes make typecheck clean. make check: 3550 passed.
padak
left a comment
There was a problem hiding this comment.
Review of #310 — feat(0.44.0): kbagent agent -- CLI parity for /agents REST surface
Generated by
kbagent-pr-reviewersubagent (second pass / delta mode). 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 the second-pass review of PR #310. The prior review (REQUEST CHANGES: B-1 duplicate
changelog key, NB-1 wrong version tags, NB-4 missing --id alias tests, NIT-1 lambdas not
extracted to named renderers) has been fully addressed. All four prior issues are resolved;
the consolidation from 0.44.0b2 to a single stable 0.44.0 entry is clean, version tags on
gotchas.md and commands-reference.md now correctly say v0.44.0, --id/--task-id/--run-id
alias tests cover every ID alias path, and the run/test/prompt-improve lambdas were
extracted to named functions. The two remaining lambdas in agent.py (lines 689 and 833) are
consistent with the grandfathered lambda c, d: c.print(...) pattern used pervasively in
branch.py, config.py, data_app.py, and others -- not a violation. make check passes
(3550 passed, 7 skipped). The file at 966 LOC is over the 800-LOC soft ceiling but well
under the 1200-LOC hard ceiling; CONTRIBUTING.md explicitly states the split obligation
triggers on the next PR that adds material, not on this one. No new BLOCKING issues
were found in this pass. Verdict: APPROVE.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 1
- Nits: 2
Blocking findings
(none)
Non-blocking findings
[NB-1] src/keboola_agent_cli/commands/agent.py:689 — delete renderer is an assigned lambda with domain meaning
formatter.output({"status": "deleted", "id": task_id}, lambda c, d: c.print(f"[bold green]Deleted[/bold green] task [cyan]{d['id']}[/cyan]")) at line 689 is technically a
single-expression lambda passed inline, but it carries domain meaning (styled "Deleted task"
banner) that CONTRIBUTING.md's named-function rule targets ("domain meaning -- gets a named
def"). The analogous _render_created_task / _render_updated_task named functions exist
a few dozen lines above for exactly this shape (line 290-300). This is minor and consistent
with a widespread grandfathered pattern in the codebase, but the named-renderer path was
specifically the NIT-1 fix from the prior review -- completing the extraction would be
fully consistent with the spirit of that fix.
Fix (optional): extract to def _render_deleted_task(c: Any, d: dict[str, Any]) -> None: c.print(f"[bold green]Deleted[/bold green] task [cyan]{d['id']}[/cyan]").
[NB-2] src/keboola_agent_cli/commands/agent.py:833 — no-op lambda on json-mode-guarded branch never reaches the human formatter
At line 832-834: if formatter.json_mode: formatter.output({...}, lambda c, d: None); return. The lambda c, d: None human formatter is dead code -- formatter.output() in
json_mode never calls the human callback, and the return immediately after means the
else branch (lines 835-836) handles the non-json path separately. The lambda occupies
a parameter slot it can never fill. Passing None as the human_formatter argument or using
an explicit named no-op makes the intent clearer and eliminates the meaningless closure.
Fix (optional): replace with formatter.output({"events": events, "count": len(events)}) if
OutputFormatter.output() accepts a missing/None second argument, or extract a module-level
_noop_renderer: Callable[[Any, Any], None] = lambda c, d: None that documents the intent.
This is the same pattern as lambda c, d: None already used in the _render_run_result
group -- a single-file constant would avoid the duplication across both call sites.
Nits
-
[NIT-1]src/keboola_agent_cli/commands/agent.py:966-- file is at 966 LOC, 21%
over the 800-LOC soft ceiling. No action required before merge per CONTRIBUTING.md
("soft ceiling: the next PR that adds material should split first"). Worth noting as a
future split target: the natural boundary is_render_*helpers (lines 220-462,
~240 LOC) into an_agent_renderers.pysibling, reducing the main file to roughly 720
LOC and keeping it clean for the next command addition. -
[NIT-2]tests/test_agent_cli.py:31 test functions-- the test count in the PR
description says "13 CLI tests" butgrep -c '^ def test_'returns 31. The count
discrepancy is not a correctness issue (having more tests is better) but creates a minor
misleading impression in the description for reviewers verifying coverage claims. No
action needed.
Verification log
gh pr view 310 --json title,state,files-> 31 files, +3830/-813, state=OPEN,
feat(0.44.0):conventional commit prefix, typefeatmatches new-behavior change.git rev-parse --abbrev-ref HEAD->feat/agent-cli-parity(matches<branch>).cat src/keboola_agent_cli/changelog.py | grep '"0.44.0"\|"0.42.0"'->
"0.44.0"key present with single stable consolidated entry;
"0.42.0"key present with workspace discoverability fix only -- NO agent content.
B-1 from prior review confirmed resolved.grep 'since v0.44\|since v0.42' plugins/.../gotchas.md | grep -i agent->
(since v0.44.0)on both agent gotcha entries (lines 1998, 2023).
(since v0.42.0)absent from agent entries. NB-1 from prior review confirmed resolved.grep 'Agent Tasks' plugins/.../commands-reference.md->
## Agent Tasks (since v0.44.0)at line 240. Version tag correct.grep '\-\-id\|\-\-task-id\|\-\-run-id' tests/test_agent_cli.py->
lines 511, 517, 523, 529, 545, 547, 556, 562 -- all six ID alias forms covered
including conflict/missing-id paths. NB-4 from prior review confirmed resolved.grep 'lambda' src/keboola_agent_cli/commands/agent.py->
2 remaining lambdas at lines 689, 833 (see NB-1/NB-2 above);
run/test/prompt-improve lambdas extracted to named_render_*functions at lines
302, 313, 322. NIT-1 from prior review confirmed resolved.grep 'ty: ignore' src/keboola_agent_cli/commands/agent.py-> line 180 only, with
one-line rationale comment (trigger_on constrained to Literal set by CLI/REST boundary).grep 'ty: ignore' src/keboola_agent_cli/services/agent_service.py-> line 267 only,
with rationale comment (_NullStore is no-op stand-in; store argument unused in test_action).make typecheck 2>&1 | grep 'commands/agent\|services/agent_service\|server/agents_store'->
empty. No ty diagnostics on any of the new agent files. Bothty: ignoreannotations
have required rationale comments.grep -n '@agent_app.command' src/.../commands/agent.py | wc -l-> 12 commands.
grep 'agent\.' src/.../permissions.py | wc -l-> 12 entries. 1:1 match.- Layer violation checks:
grep typer/click in services/agent_service.py-> empty (no violation).grep httpx in commands/agent.py-> empty (no violation).grep FastAPI in server/agents_store.py-> empty (PEP 562 shim correctly isolates).
cat src/.../hints/definitions/agent.py-> intentionally empty, with documented
rationale.CONTRIBUTING.mdexception clause covers pure-infrastructure commands.grep '@router\.' src/.../server/routers/agents.py | wc -l-> 14 HTTP routes covering
list, create, get, update, delete, run (blocking + SSE), list-runs, get-run, get-run-events,
test (blocking + SSE), cron-preview, prompt-improve (blocking + SSE). CLI-to-REST 1:1
parity confirmed.make check-> 3550 passed, 7 skipped. Exit 0.wc -l src/.../commands/agent.py-> 966 LOC (over 800 soft, under 1200 hard ceiling).- Version sync:
pyproject.toml=0.44.0,plugin.json=0.44.0,
marketplace.json keboola-agent-cli entry=0.44.0,uv.lock self-pin=0.44.0. All consistent. - Behavior verification: could not run
kbagent agent cron-previewlive -- not in scope for
second-pass delta review since E2E tests inTestE2EAgentTaskscover the full
create-show-update-run-runs-delete lifecycle and are listed as passing in the PR description.
Open questions for the author
(none)
…310 re-review) Re-review (APPROVE) flagged two optional cleanups: - delete confirmation lambda -> named _render_deleted_task (consistent with the create/update/run/test/prompt-improve renderers). - run-events json-mode branch passed a no-op lambda c, d: None; human_formatter is never called in json mode, so drop it (output() defaults to None). agent.py now has zero lambdas -- every renderer is a named function. PR description test count corrected (13 -> 31 CLI tests). make check: 3550 passed.
padak
left a comment
There was a problem hiding this comment.
Review of #310 — feat(0.44.0): kbagent agent — CLI parity for /agents REST surface
Generated by
kbagent-pr-reviewersubagent (delta re-review pass). 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
Delta pass confirming resolution of the three findings addressed since the APPROVE pass (commit 1c4f451). All three fixes are verified clean; the only open item is the intentionally deferred file-size NIT. No new issues found.
Verdict: APPROVE. make check passes at 3550/7, GitHub CI (check + Windows wheel build) is green, ty check is clean on commands/agent.py, and zero lambdas remain in that file.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 0
- Nits: 1 (carried over, intentionally deferred)
Blocking findings
(none)
Non-blocking findings
(none)
Nits
[NIT-1]src/keboola_agent_cli/commands/agent.py(968 LOC) — File is above the 800-line soft ceiling documented in CONTRIBUTING.md. This is intentionally NOT addressed in this PR: the ribbon rule defers the split obligation to the next PR that adds material to the file, and this PR creates (not extends) the file. No action required now; the split obligation applies to whoever next adds substantial content.
Verification log
gh auth status-> padak@github.com active, token scopes sufficientgit rev-parse --abbrev-ref HEAD->feat/agent-cli-parity(matches PR branch)gh pr view 310 --json state,title,files-> OPEN, 31 files, +3832/-813gh pr checks 310->checkPASS (3m0s),Windows wheel buildPASS (1m22s)grep -n 'lambda' commands/agent.py-> empty (zero lambdas remain) [NB-1 confirmed fixed]grep -n '_render_deleted_task' commands/agent.py-> line 302, named function present [NB-1 confirmed fixed]commands/agent.py:835->formatter.output({"events": events, "count": len(events)})with no second argument; relies onhuman_formatter=Nonedefault [NB-2 confirmed fixed]wc -l commands/agent.py-> 968 lines [NIT-1 still present, deferred by design]uv run ty check src/keboola_agent_cli/commands/agent.py->All checks passed!make check->3550 passed, 7 skipped, 14 warningsin 65.53s, exit 0
Open questions for the author
(none)
Summary
Twelve
kbagent agent <verb>subcommands matching the/agentsREST endpoints thatkbagent servehas exposed since v0.40.0. CRUD (list,show,create,update,delete), execution (run [--stream] [--runtime-prompt]), history (runs,run-detail,run-events), and utilities (test [--stream],cron-preview,prompt-improve [--stream/--no-stream]).Pure-local on
<config_dir>/agents.json-- CRUD + ad-hocrunwork offline. The cron loop that fires scheduled tasks still requireskbagent serverunning, but the on-disk format is identical, so a CLI-created task fires on its cron as soon as the server boots and reads the file.Closes the gap between the React UI sidebar "Agent Tasks" (which had this surface since v0.40.0) and CLI users who had to fall back to
kbagent http <verb> /agents...from inside scheduled subprocesses.Key design choices
validate_trigger(cycle / self-loop check) andmerge_runtime_input(per-action-type runtime input merge) extracted fromrouters/agents.pyintoserver/agents_store.py. The router wraps the new helpers inHTTPException(422); the CLI service wraps them inConfigError-- exact same checks, no drift.CliAgentRegistry.run_task_onceonly needsconfig_store+mcpfrom a registry; we build a tiny dataclass instead of standing up the full FastAPIServiceRegistry(which would instantiate 25+ services for a single CLI run).server/__init__.pysplit into PEP 562 shim +server/app.py. Importingfrom keboola_agent_cli.server.agents_store import AgentStoreno longer drags FastAPI/uvicorn -- the agent CLI works on plain installs without[server]extras.cronitermoved from[server]extras to core deps.cron-previewandcompute_next_runneed it outside serve.agent.*. Agent CRUD is pure-local file I/O; there is no Keboola HTTP API to "hint" at. The rationale is documented inhints/definitions/agent.py.Test plan
make lint format-check changelog-check test(3375 passed, 7 skipped)test_agent_service.py(CRUD round-trip, cron validation, trigger validation incl. self-loop + missing target, runtime_input merge, run mocking, stream test action, run history NOT_FOUND mapping)test_agent_cli.py(every subcommand viaCliRunneragainst a real tmp config dir, both human +--jsonmodes, runtime-prompt merge path)tests/test_e2e.py::TestE2EAgentTasks(cron-preview, full create-show-update-run-runs-delete lifecycle with cli_command action, ad-hoc test)kbagent agent cron-preview --cron "0 6 * * 1"-> next 3 firingskbagent agent create --name X --cron "..." --type cli_command --argv version-> persistedkbagent agent list / show / update / delete-> round-trips in both human +--jsonmodeskbagent agent test --type cli_command --argv version-> ad-hoc run, no persistencemake version-sync+make skill-genregeneratedplugin.jsonand the auto-generated SKILL.md tabletest_permissions.py::test_all_subapp_commands_registeredpassesagent_runner/agents_store/pricing/run_broadcaster(58 tests) still pass against the refactored importskeboola-expert.md<= 60 KB after additions)Documentation sync (silent-drift surfaces)
Every file mandated by
CONTRIBUTING.md"Plugin synchronization map" +CLAUDE.mdconvention #17 was updated:CLAUDE.md## All CLI Commands-- fullagentcheat sheetcommands/context.pyAGENT_CONTEXT-- per-subcommand documentationkeboola-expert.md-- Rule 6 VERSION GATE entry + Tool Selection Matrix rowSKILL.md-- auto-regenerated decision table + manual workflow rowscommands-reference.md-- new "Agent Tasks (since v0.42.0)" sectiongotchas.md-- two new(since v0.42.0)entriesagent-tasks-cli-workflow.mdskill reference (CLI-first walkthrough)agent-tasks-workflow.md->agent-tasks-rest-workflow.md(kept for AI-agent subprocess REST callbacks)plugin.json/marketplace.jsonre-synced