feat(data-app): git-repo introspection + managed-repo credentials (0.61.0) - #414
Conversation
| @router.get("/{project}/{app_id}/git-repo", summary="Get a data app's git repository") | ||
| def git_repo( | ||
| project: str, app_id: str, registry: ServiceRegistry = Depends(get_registry) | ||
| ) -> dict[str, Any]: | ||
| """Show clone URLs of the app's configured git repo. Mirrors `kbagent data-app git-repo`. | ||
|
|
||
| Returns 409 until the app has been deployed at least once (the git block is | ||
| synced from the Storage config into the Data Science app record at deploy | ||
| time). | ||
| """ | ||
| return registry.data_app.get_data_app_git_repo(alias=project, app_id=app_id) | ||
|
|
||
|
|
||
| @router.get("/{project}/{app_id}/git-repo/branches", summary="List a data app's git branches") | ||
| def git_branches( | ||
| project: str, app_id: str, registry: ServiceRegistry = Depends(get_registry) | ||
| ) -> dict[str, Any]: | ||
| """List remote branches of the app's git repo. Mirrors `kbagent data-app git-branches`.""" | ||
| return registry.data_app.list_data_app_git_branches(alias=project, app_id=app_id) | ||
|
|
||
|
|
||
| @router.get("/{project}/{app_id}/git-repo/entrypoints", summary="List a data app's git entrypoints") | ||
| def git_entrypoints( | ||
| project: str, app_id: str, registry: ServiceRegistry = Depends(get_registry) | ||
| ) -> dict[str, Any]: | ||
| """List root-level .py entrypoints. Mirrors `kbagent data-app git-entrypoints`.""" | ||
| return registry.data_app.list_data_app_git_entrypoints(alias=project, app_id=app_id) | ||
|
|
||
|
|
||
| @router.get("/{project}/{app_id}/git-repo/credentials", summary="List managed git credentials") | ||
| def git_credentials( | ||
| project: str, app_id: str, registry: ServiceRegistry = Depends(get_registry) | ||
| ) -> dict[str, Any]: | ||
| """List credentials of the app's MANAGED git repo. Mirrors `kbagent data-app git-credentials`. | ||
|
|
||
| The credential secret is never returned here; needs an admin storage token. | ||
| """ | ||
| return registry.data_app.list_data_app_git_credentials(alias=project, app_id=app_id) | ||
|
|
||
|
|
||
| @router.post("/{project}/{app_id}/git-repo/credentials", summary="Create a managed git credential") | ||
| def git_credentials_create( | ||
| project: str, | ||
| app_id: str, | ||
| body: GitCredentialCreate, | ||
| registry: ServiceRegistry = Depends(get_registry), | ||
| ) -> dict[str, Any]: | ||
| """Mint a git credential for the app's MANAGED git repo. | ||
|
|
||
| Mirrors `kbagent data-app git-credentials-create`. For ``type=http_token`` | ||
| the response carries a one-time ``secret``. Needs an admin storage token; | ||
| external repos return 409. | ||
| """ | ||
| return registry.data_app.create_data_app_git_credential( | ||
| alias=project, | ||
| app_id=app_id, | ||
| type_=body.type, | ||
| permissions=body.permissions, | ||
| public_key=body.public_key, | ||
| name=body.name, | ||
| ) |
There was a problem hiding this comment.
🚩 REST router git-repo routes don't conflict with existing path-parameter routes
The new routes at src/keboola_agent_cli/server/routers/data_apps.py:324-384 (e.g., GET /{project}/{app_id}/git-repo) are correctly distinguished from the existing GET /{project}/{app_id} route (line 100) by Starlette's path matching — {app_id} is a single-segment str parameter, so /myproj/123/git-repo matches the more-specific literal-suffix route, not the two-segment catch-all. However, the pre-existing POST /validate-repo (line 302) and POST /{project} (line 111) are a latent conflict where a POST to /validate-repo could theoretically match /{project} first with project="validate-repo". This is a pre-existing issue unrelated to this PR, but worth noting for future route restructuring.
Was this helpful? React with 👍 or 👎 to provide feedback.
padak
left a comment
There was a problem hiding this comment.
Review of #414 — feat(data-app): git-repo introspection + managed-repo credentials (0.61.0)
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 five data-app git-* commands (git-repo, git-branches, git-entrypoints, git-credentials, git-credentials-create) that wrap the sandboxes-service /apps/{id}/git-repo/* endpoints. The implementation follows the 3-layer pattern correctly, all plugin synchronization surfaces in the checklist are updated (OPERATION_REGISTRY, CLAUDE.md, context.py, commands-reference.md, gotchas.md, data-app-workflow.md, SKILL.md table, permissions.py), and the test suite is comprehensive (25 unit/CLI/client tests + 1 E2E test). The one genuine finding is that both commands/data_app.py and services/data_app_service.py were already above their CONTRIBUTING.md hard ceilings before this PR, and this change widens the gap further without the required pre-split. keboola-expert.md §3 inline gotchas was not updated for the deploy-once precondition, though it is documented in gotchas.md. Verdict: REQUEST CHANGES (one blocking file-size violation).
Verdict
- Verdict: REQUEST CHANGES
- Blocking findings: 1
- Non-blocking findings: 2
- Nits: 2
Blocking findings
[B-1] src/keboola_agent_cli/commands/data_app.py and src/keboola_agent_cli/services/data_app_service.py — file-size hard ceilings exceeded; splitting required before merging
CONTRIBUTING.md § "File-size budgets" states: "When a file crosses the hard ceiling, splitting is required before merging more functionality into it." Hard ceilings are 1200 LOC for commands/ and 1500 LOC for services/.
commands/data_app.pywas 1271 LOC onmain(already above the 1200 hard ceiling); this PR adds 320 LOC, bringing it to 1591 LOC.services/data_app_service.pywas 2082 LOC onmain(already above the 1500 hard ceiling); this PR adds 184 LOC, bringing it to 2266 LOC.
Both files were in violation before this PR, but the rule is explicit: you must split before adding more functionality to a file above the hard ceiling, not after. The suggested split path from CONTRIBUTING.md for commands: extract the git-* commands into commands/_data_app_git.py and import into data_app_app in data_app.py. For services: extract the git-* methods (lines 1033–1208) plus _normalize_git_credential into services/data_app_git_service.py.
Non-blocking findings
[NB-1] plugins/kbagent/agents/keboola-expert.md:238 — §3 Inline Gotchas "Data apps" section not updated with deploy-once precondition
The deploy-once precondition for git-repo / git-branches / git-entrypoints (the three introspection commands return 409 until the app has been deployed at least once) is non-obvious and is documented in gotchas.md (line 2610) and commands-reference.md (line 192), but was NOT added to keboola-expert.md §3 "Data apps" inline gotchas. The keboola-expert subagent system prompt is what the specialist agent reads at runtime before its first action; missing it means the agent will not warn the user before issuing a git-repo call on a freshly created --no-deploy app, then receive a 409, and fall back to an explanation that should have been upfront.
Fix: add a one-liner under the "Data apps" section of §3, e.g.:
- **git-repo/git-branches/git-entrypoints** (0.61.0+): return 409 "no Git repository configured" until the app has been deployed at least once -- run data-app deploy first.
[NB-2] plugins/kbagent/skills/kbagent/SKILL.md:415 — bottom workflow-table entry for data-app-workflow not updated
The bottom reference table (line 415) still reads: "Data apps (create / deploy / start / stop / password / delete; the §9 redeploy contract)" — it does not mention the new git-* introspection and credential commands. A reader scanning the table to decide whether to open the workflow file will not know git operations are covered there.
Fix: extend the description in the pipe-table cell to include "git-repo introspection, managed-repo credentials", e.g.:
| **Data apps** (create / deploy / start / stop / password / delete; git-repo introspection; managed-repo git credentials; the §9 redeploy contract) | [data-app-workflow](references/data-app-workflow.md) |
Nits
-
[NIT-1]plugins/kbagent/skills/kbagent/references/commands-reference.md:193— the entry fordata-app git-branchessays "raw top-level array from the server"; this describes the upstream API response shape, not whatkbagent --jsonactually emits (which is{"data": {"project_alias": ..., "app_id": ..., "branches": [...], "count": N}}). Consider rephrasing to "server returns a raw top-level array; kbagent normalizes it into{branches: [...], count: N}" to avoid confusing downstream consumers who pattern-match on the JSON output. -
[NIT-2]plugins/kbagent/skills/kbagent/SKILL.md:3— thedescription:trigger-keyword block (lines 3–68) was not extended with new topics likegit-repo,managed git credential,git credentials,ssh_key credential,http_token credential. The SKILL.md decision table was correctly updated (5 new rows), so auto-dispatch via the table works, but keyword-based trigger matching in Claude's skill routing will not fire for raw "data app git repo" queries unless they also mention "data app" or another existing keyword. Low-risk since "data app" is already listed, but adding a few keywords would improve discoverability.
Verification log
gh pr view 414 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ 18 files, +1556/-4, conventionalfeat(data-app):✓, state OPEN ✓git rev-parse --abbrev-ref HEAD→claude/gallant-dirac-ccb6e8matches PR branch ✓Read CONTRIBUTING.md→ loaded plugin synchronization map and file-size budget table ✓Read plugins/kbagent/agents/keboola-expert.md→ loaded §1 rules, §2 matrix, §3 gotchas ✓- Layer violation grep (typer in services, httpx in commands, formatter in clients) → empty ✓ (no violations)
- Raw error_code string literal grep → empty ✓
- Bare
except:grep → empty ✓ print()in production code grep → empty ✓- Magic-number grep → empty ✓
- Token-in-log grep (without mask_token) → no hits in production paths ✓
OPERATION_REGISTRYcheck → 5 entries found (data-app.git-repo,.git-branches,.git-entrypoints,.git-credentials,.git-credentials-create) ✓CLAUDE.md ## All CLI Commands→ 6 new lines present (5 commands + inline comment) ✓commands/context.pyAGENT_CONTEXT→ grep confirmed 5 new commands documented ✓commands-reference.md→ 5 new bullet entries confirmed ✓gotchas.md→ new section at line 2610 with(since v0.61.0)version tag ✓data-app-workflow.md→ 49 new lines with git-repo and git-credentials sections ✓SKILL.mddecision table → 5 new rows confirmed ✓permissions.py→ 5 entries confirmed (4 read + 1 write) ✓keboola-expert.md→ NOT in changed files list; §3 data-apps gotchas NOT updated → [NB-1]keboola-expert.mdbyte count → 49523 bytes (well under the 62000 byte hard cap) ✓keboola-expert.md§2 matrix → no new group row needed; git-* commands are part of existingdata-appgroup ✓ (per CONTRIBUTING.md: "Adding a command to an existing group needs no new row")commands/data_app.pyLOC: main=1271, PR=1591 (hard ceiling 1200) → [B-1]services/data_app_service.pyLOC: main=2082, PR=2266 (hard ceiling 1500) → [B-1]tests/test_data_app_git_repo.py→ 25 test methods across 3 classes (TestGitRepoService, TestGitRepoCli, TestGitRepoClient);ds.close.assert_called_once()present in service tests ✓- E2E test
test_data_app_git_repo_introspection→ confirmed intests/test_e2e.py✓ make check→ 3999 passed, 8 skipped (exit 0) ✓- Behavior reproduction: not attempted (no live data app with managed git repo available in the reviewer's environment; behavior claims accepted from PR description as author-verified against a real project)
serveREST routes → 5 new routes confirmed inserver/routers/data_apps.py(GET git-repo, GET branches, GET entrypoints, GET credentials, POST credentials); 1:1 parity with CLI commands ✓- Service-layer validation (type/permissions mutex for
create_data_app_git_credential) → confirmed atdata_app_service.py:1137–1162; also duplicated at command layer for CLI exit-2 UX; serve route correctly relies on service validation ✓ - JSON confirmation bypass in
git-credentials-create→ confirmed atcommands/data_app.py:1548:not formatter.json_mode→ json callers skip prompt ✓
Open questions for the author
(none)
…iew #414) Address the file-size-budget blocking finding from the PR review: both commands/data_app.py and services/data_app_service.py were already over their CONTRIBUTING.md hard ceilings, so the new git-* functionality moves into dedicated modules instead of growing them further: - services/data_app_git_service.py -- new DataAppGitService (the 5 git-repo methods + credential normalizer), wired into cli.py ctx + serve registry. - commands/_data_app_git.py -- the 5 `data-app git-*` commands, attached to the data-app sub-app via register_git_commands(); they still surface as `kbagent data-app git-*` with unchanged names/permissions. - data_app.py / data_app_service.py return to their pre-feature size. Also address the two non-blocking doc findings: add the deploy-once precondition + managed-only credential note to keboola-expert.md (matrix row + inline gotcha) and mention git-* in the SKILL.md data-app workflow-table row. No behavior change: command names, permissions, serve REST routes, and response shapes are identical. 3999 tests pass; ruff / ty / command-sync / changelog all green.
|
Addressed the review findings in 9dc460d: 🔴 Blocking — file-size budget. Both
Command names, 🟡 Non-blocking #1 — 🟡 Non-blocking #2 —
|
3a8f821 to
13b8d50
Compare
padak
left a comment
There was a problem hiding this comment.
Review of #414 — feat(data-app): git-repo introspection + managed-repo credentials (0.61.2)
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 five data-app git-* commands that expose the sandboxes-service /apps/{id}/git-repo/* surface: three read-only introspection commands (git-repo, git-branches, git-entrypoints) and two credential-management commands for managed repos (git-credentials, git-credentials-create). It is a re-review following a prior round that flagged file-size violations. The author correctly resolved the prior BLOCKING finding by splitting the new git service logic into dedicated modules (commands/_data_app_git.py + services/data_app_git_service.py) instead of adding to the already-over-budget data_app.py / data_app_service.py. All six silent-drift surfaces from CONTRIBUTING.md §Plugin synchronization map are updated, make check passes cleanly (4026 tests, 0 failures), and the keboola-expert.md byte budget is at 51,331 / 62,000 (comfortable headroom). Verdict: APPROVE. There is one NON-BLOCKING finding (the pre-existing data_app.py hard-ceiling overshoot) and one NIT.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 1
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] src/keboola_agent_cli/commands/data_app.py:1279 — pre-existing hard-ceiling breach not resolved by the split
data_app.py sits at 1,279 LOC on main (1,271 before this PR; this PR added 8 lines for the import and register_git_commands(data_app_app) call). The CONTRIBUTING.md hard ceiling for commands/*.py is 1,200 LOC. The prior review correctly identified the need to split; the author split the new git commands into _data_app_git.py, which is the right response. However, the host module itself remains above the hard ceiling. The 8 lines this PR adds are strictly mechanical wiring (an import and a function call), not substantive additions, so blocking on it now would be disproportionate — but the file needs a dedicated split pass before the next material addition to data_app.py.
Fix: open a follow-up issue to split data_app.py (e.g., extract data-app secrets-* into _data_app_secrets.py in the same pattern as this PR's _data_app_git.py). The author's docstring in _data_app_git.py already acknowledges the budget concern, which is good.
Nits
[NIT-1]src/keboola_agent_cli/commands/data_app.py:1274-1276— the comment block aboveregister_git_commands(data_app_app)contains a blank line between the import block and the comment (from ._data_app_git import register_git_commandsat line 24, then the comment block at ~line 1274). The comment says "Attach the data-app git-* commands" — this is fine prose but slightly longer than needed given the docstring in_data_app_git.pyalready explains the rationale. A one-liner# attach git-* commands (see _data_app_git.py)would be tighter, though this is purely stylistic.
Verification log
gh pr view 414 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ 22 files, +1652/-9,feat(data-app):conventional prefix, state=OPEN ✓git rev-parse --abbrev-ref HEAD→claude/gallant-dirac-ccb6e8matches<branch>✓wc -l commands/_data_app_git.py services/data_app_git_service.py data_science_client.py→ 343 / 224 / 342 LOC, all within layer budgets ✓wc -l commands/data_app.py→ 1,279 LOC (1,271 onmain; hard ceiling 1,200); pre-existing, only +8 mechanical lines in this PR — see NB-1wc -l services/data_app_service.py→ 2,082 LOC onmainAND this PR (file unchanged by this PR) ✓ PR correctly avoided adding to itgrep typer services/data_app_git_service.py→ empty; no layer violation ✓grep httpx commands/_data_app_git.py→ empty; no direct HTTP in commands ✓- Layer 3 (
data_science_client.py): 5 new methods (get_git_repo,list_git_branches,list_git_entrypoints,list_git_credentials,create_git_credential) all route via_do_request(inherited retry/backoff) ✓ permissions.py OPERATION_REGISTRY→ 4read+ 1writeentries for all 5 new commands ✓server/routers/data_apps.py→ 5 matching REST routes (GET git-repo,GET git-repo/branches,GET git-repo/entrypoints,GET git-repo/credentials,POST git-repo/credentials) with correctGitCredentialCreatebody model (type, permissions, public_key, name) ✓cli.py→DataAppGitServiceimported, instantiated, and wired intoctx.obj["data_app_git_service"]✓commands/context.py AGENT_CONTEXT→ all 5 commands documented with deploy-once precondition and gotcha ✓; 4 pre-existing{id}→{{id}}brace-escape fixes bundled in same diff ✓CLAUDE.md ## All CLI Commands→ 5 new command signatures + inline comment block added ✓keboola-expert.md→ §2 Tool Selection Matrix: new row "Inspect / manage the git repo of an EXISTING data app" (0.61.2+) ✓; §3 Inline Gotchas: deploy-once precondition, managed-only constraint, one-time secret warning ✓; byte size 51,331 / 62,000 limit ✓plugins/kbagent/skills/kbagent/SKILL.md→ 5 new decision-table rows (4 read + 1 write) + updated "Data apps" workflow description ✓plugins/kbagent/skills/kbagent/references/commands-reference.md→ 5 new bullets withsince 0.61.2tags ✓plugins/kbagent/skills/kbagent/references/gotchas.md→ new section## data-app git-repo / git-branches / git-entrypoints need a deployed app (since v0.61.2)with(since v0.61.2)version tag ✓plugins/kbagent/skills/kbagent/references/data-app-workflow.md→ "Inspect the deployed-from git repo (since v0.61.2)" and "Manage git credentials for a managed repo (since v0.61.2)" sections ✓server/dependencies.py→DataAppGitServiceregistered inServiceRegistry✓- Security:
_normalize_git_credential()deliberately omitssecret;create_data_app_git_credential()re-attaches it only forhttp_token(one-time surface);git-credentialslist endpoint never returns the secret ✓ - Secret surfacing in
_data_app_git.pycommand: secret printed inline in human mode and present in JSON output — intentional, consistent withdata-app passwordpattern ✓ grep -E 'bare except|print\(|error_code="[A-Z_]+"' diff→ empty ✓test_data_app_git_repo.py→ 610 LOC, 25 test functions across service, CLI (CliRunner), and HTTP-client (httpx_mock) layers; includestest_list_credentials_never_leaks_secretandtest_create_http_token_returns_one_time_secret✓- E2E test
test_data_app_git_repo_introspectionintests/test_e2e.py(create → deploy → poll git-repo → branches/entrypoints/credentials) ✓ make check(in worktree) → 4,026 passed, 8 skipped, 0 failures ✓ (All checks passed!)wc -c keboola-expert.md→ 51,331 bytes (budget 62,000) ✓pyproject.toml version = "0.61.2"matchesplugin.json "version": "0.61.2"✓- Behavior reproduction: could not run live (no data-app with a deployed git repo in local config). Author states in PR description that behavior was verified live. The deploy-once precondition and 409-on-external-repo paths are documented in both
gotchas.mdand--helptext; noted as "author-verified, not independently reproduced" in this run.
13b8d50 to
a1f756e
Compare
ce4db1f to
8e03b74
Compare
…63.3)
Add five `data-app git-*` commands wrapping the sandboxes-service /apps/{id}/git-repo/* endpoints, in dedicated command + service modules (commands/_data_app_git.py + services/data_app_git_service.py):
- git-repo / git-branches / git-entrypoints -- read-only introspection (clone URLs + managed flag, remote branches with commit metadata, root-level .py entrypoints). Project storage token only.
- git-credentials / git-credentials-create -- list and mint SSH-key / HTTP-token credentials for a *managed* git repo (admin storage token; http_token returns a one-time secret shown once).
3-layer: DataScienceClient -> DataAppGitService -> commands/_data_app_git.py (register_git_commands), plus OPERATION_REGISTRY entries (4 read, 1 write) and 1:1 serve REST routes. The git-* code lives in its own modules so the new functionality stays out of the already-large data_app.py / data_app_service.py.
Gotcha verified live: introspection returns 409 until the app has been deployed at least once (git block syncs Storage->DS at deploy); credentials are managed-repo only, so external repos from `data-app create --git-repo` return 409 on create.
Also fixes a pre-existing rendering bug in commands/context.py: AGENT_CONTEXT is an f-string, so unescaped {id} placeholders rendered as '<built-in function id>' in `kbagent context`; literal braces are now escaped.
Tests: tests/test_data_app_git_repo.py (service + CLI + client HTTP) + an E2E introspection case. Docs synced (CLAUDE.md, context.py, commands-reference.md, gotchas.md, data-app-workflow.md, keboola-expert.md, SKILL.md). Rebased onto main 0.63.2; version -> 0.63.3.
8e03b74 to
f40095e
Compare
What
Adds five
data-app git-*commands that wrap the Keboola sandboxes-service/apps/{appId}/git-repo/*endpoints, so you can inspect and manage the git repository a data app is deployed from:data-app git-repoGET /apps/{id}/git-repodata-app.git-repodata-app git-branchesGET .../git-repo/branchesdata-app.git-branchesdata-app git-entrypointsGET .../git-repo/entrypointsdata-app.git-entrypointsdata-app git-credentialsGET .../git-repo/credentialsdata-app.git-credentialsdata-app git-credentials-createPOST .../git-repo/credentialsdata-app.git-credentials-create.pyentrypoints). Project storage token only.http_tokenreturns a one-time secret printed once (mirrorsdata-app password),ssh_keyrequires a public key.Why
Closes the gap between
data-app create --git-*(write-once git config at create time) andvalidate-repo(pre-create repo check): these commands inspect and manage the git connection of an existing app server-side.How
Standard 3-layer extension, no new client and no new
cli.pywiring:data_science_client.py— 5 methods (raw-array shapes handled defensively).data_app_service.py— 5 methods (camelCase→snake_case, service-leveltype/public_keymutex validation for the serve route).data_app.py— 5 thin Typer commands (dual JSON/Rich output, one-time-secret print, usage-error mutex).permissions.pyOPERATION_REGISTRY (4 read, 1 write) and 1:1serveREST routes inserver/routers/data_apps.py.Reviewer notes (verified live against a real project)
--no-deployapp has no git repo from the service's point of view. Documented ingotchas.md+ changelog +--help.data-app create --git-repo <url>are external, sogit-credentials-createreturns 409 "no managed Git repository" for them; credential management targets managed repos (provisioned in the UI). Thegit-credentialslist returns an empty list (200) for external repos.git-branchesis a raw top-level array,git-entrypointsa rawarray<string>(extension hardcoded to.pyserver-side).Tests & docs
tests/test_data_app_git_repo.py— 25 tests across service, CLI, and client-HTTP (httpx_mock) layers, incl. secret-never-leaks-in-list and the type/public-key mutex.tests/test_e2e.py(create → deploy → poll git-repo → branches/entrypoints/credentials).CLAUDE.md,context.py,commands-reference.md,gotchas.md,data-app-workflow.md,SKILL.md(regenerated).0.60.3→0.61.0(changelog + version-sync).Full suite: 3999 passed, 133 skipped;
ruff/ruff format/ty/check_command_sync/check_error_codes/changelog-checkall green.