Skip to content

feat(data-app): AI-3753 grant Storage access by default on data-app create - #626

Merged
MiroCillik merged 4 commits into
mainfrom
miro-ai-3753-data-app-create-workspace-flag
Aug 21, 2026
Merged

feat(data-app): AI-3753 grant Storage access by default on data-app create#626
MiroCillik merged 4 commits into
mainfrom
miro-ai-3753-data-app-create-workspace-flag

Conversation

@MiroCillik

@MiroCillik MiroCillik commented Aug 21, 2026

Copy link
Copy Markdown
Member

Closes AJDA-3183.

kbagent data-app create gains --workspace / --no-workspace, defaulting ON. It writes runtime.workspace.enabled: true — the single switch that makes the platform provision a data app's ephemeral workspace and inject WORKSPACE_ID, QUERY_SERVICE_URL and KBC_WORKSPACE_MANIFEST_PATH.

Validation of the report

Every claim in the issue checks out:

Claim Verified
kbagent never sets the flag data_app_service.py wrote "runtime": {"backend": {"size": size}} and nothing else; grep for workspace.enabled across the repo returned 0 hits
MCP does set it keboola-mcp-server tools/data_apps.py:1242, gated on has_feature('data-apps-storage-workspace')
MCP has a legacy fallback :1113-1117 injects WORKSPACE_ID via parameters.dataApp.secrets from a find-or-create RO workspace
The config shape live Direct Access Beers JS config in project 40: runtime: {backend: {size, type}, workspace: {enabled: true}}

Two decisions worth reviewing

1. Default ON. Omitting the flag fails silently — the app deploys, reports state=running, passes its health probe, and reads nothing. The opposite mistake (an unused workspace on an app that never reads Storage) is the cheap one, so --no-workspace is the opt-out rather than --workspace the opt-in.

--no-workspace omits the key rather than writing enabled: false, so that request body is byte-identical to 0.86.0.

2. No feature gate, and no secrets fallback. The data-apps-storage-workspace project feature is being enabled everywhere and then removed, so the config option is the sole control. This deliberately does not copy MCP's parameters.dataApp.secrets.WORKSPACE_ID fallback — replicating it would mean creating a long-lived workspace that the app then depends on forever, and which kbagent workspace gc could delete out from under it.

Verified live

Created a real app on project 40 and read the config back:

"runtime": { "backend": { "size": "tiny" }, "workspace": { "enabled": true } }

...matching the known-working app byte for byte. (App deleted afterwards.) I verified the config shape, not a live data read — the issue already establishes causality: the reporter set this flag on their broken app and it immediately worked ([duck] cached 2412 beers in 7253ms).

Also in this PR

  • data_app_service.py was over its grandfathered size budget, so make loc-check blocked the addition. Split the pure body-builders and redaction helpers into services/_data_app_bodies.py (re-exported, so existing import sites are untouched); baseline ratcheted 1726 → 1648 code lines.
  • Fixed a latent trap in the e2e data-app cleanup fixture — it touched self._created_app_ids on teardown even when setup had skipped before assigning it, so any test gated only on credentials errored. Existing tests dodged this by carrying a skip marker, which fires before fixtures.
  • keboola-expert.md was at 61999/62000 bytes, so any addition broke its prompt budget. Trimmed back under by collapsing duplication with the §2 matrix row and honouring §3's stated "one-line triggers only" contract, rather than raising the ceiling (which the test comment explicitly discourages).

Out of scope for this repo

  • Issue item 3 (a duckdb-caching.md prerequisite line) lives in keboola/ai-kit, not here.
  • Issue item 5 (have the platform fail a deploy loudly when an app requests Storage env vars it was never granted) is a platform change. Still worth doing — it is the only fix that helps apps created by paths other than kbagent.

Test plan

  • make check — green (5699 passed)
  • Full suite with --extra server — green; confirmed the 2 new REST tests and 88 server-router tests actually executed rather than skipping
  • New coverage: 3 service tests (default on, opt-out omits the block, dry-run preview does not lie), 3 CLI tests (flag pass-through both ways, human output on/off), 2 REST tests, 2 e2e tests (live config assertion + dry-run opt-out)

Release Notes

  • Justification
    • A data app that could not read a single row reported itself healthy at every level a user can see — state=running, desiredState=running, health probe green. The one flag that grants Storage access was undocumented and unset by kbagent, and there was no CLI option to set it. Users found it by dumping the config of an unrelated working app and diffing. This makes the safe shape the default shape.
    • data-app create now grants Storage access unless you explicitly opt out with --no-workspace.
  • Plans for Customer Communication
    • Changelog entry included (0.87.0). Worth flagging to whoever owns the dataapp-developer skill: its DuckDB-cached read-only dashboard pattern is exactly the one that needs this flag, and apps created with kbagent ≤ 0.86.0 need the retrofit (config update --merge --set 'runtime.workspace.enabled=true' then data-app deploy).
    • Not a BC break: existing configs are untouched, and --no-workspace reproduces the old body exactly.
  • Impact Analysis
    • Newly created apps get an ephemeral workspace provisioned at start even when they never read Storage. That is the intended trade (cheap mistake vs. silent dead data path) and --no-workspace avoids it.
    • Not behind a feature flag. Not gated on data-apps-storage-workspace — per platform direction, that flag is being enabled everywhere and then removed.
    • No single-tenant-specific behavior. Any project where the platform ignores the flag lands on the pre-existing behavior, not worse.
  • Deployment Plan
    • Continuous — normal kbagent release. Not critical.
  • Rollback Plan
    • Two-way. Revert the commit, or pass --no-workspace. No migration, no data change, no config rewritten in place.
  • Post-Release Support Plan
    • No new support burden. If a user reports "my data app shows no data", the gotchas entry and data-app-workflow.md now name the cause and the retrofit; data-app logs shows Missing env vars: WORKSPACE_ID.
    • Support does not need a heads-up beyond the changelog.

🤖 Generated with Claude Code

…create`

`runtime.workspace.enabled: true` is the single switch that makes the
platform provision a data app's ephemeral workspace and inject
WORKSPACE_ID, QUERY_SERVICE_URL and KBC_WORKSPACE_MANIFEST_PATH. kbagent
never wrote it and offered no option to, so an app that reads Storage --
including every app built from the dataapp-developer skill's DuckDB-cached
read-only pattern -- deployed, reported state=running and passed its health
probe while unable to read a single row. The only diagnostic was
`Missing env vars: WORKSPACE_ID` buried in the container log; an app that
did not catch it crash-looped behind the probe instead.

Adds `--workspace / --no-workspace`, defaulting ON: the opposite mistake
(an unused workspace on an app that never reads Storage) is the cheap one.
`--no-workspace` omits the key rather than writing `enabled: false`, so the
request body is byte-identical to 0.86.0. The block is a sibling of
runtime.backend, matching what the UI and MCP's modify_python_js_data_app
both write.

Deliberately NOT gated on the `data-apps-storage-workspace` project
feature -- that flag is being enabled everywhere and then removed, so the
config option is the sole control. keboola-mcp-server's legacy fallback of
injecting WORKSPACE_ID via parameters.dataApp.secrets is not copied.

Mirrored on the REST surface (`"workspace": true` in POST /data-apps/{p})
and reported as `workspace` in the --json envelope; human output prints the
Storage-access state on every create, loudly when it is off.

Also:
- split the pure body-builders and redaction helpers out of
  data_app_service.py into _data_app_bodies.py (the file was over its
  grandfathered size budget); baseline ratcheted 1726 -> 1648 code lines
- fix a latent trap in the e2e data-app cleanup fixture, which errored on
  teardown for any test gated only on credentials
- trim keboola-expert.md back under its 62 kB prompt budget by collapsing
  duplication with the §2 matrix and honouring §3's "one-line triggers only"

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the data-app creation flow to grant Storage access by default by writing runtime.workspace.enabled=true into the keboola.data-apps configuration (AI-3753), with an explicit --workspace/--no-workspace opt-out. It also refactors the data-app service to stay within the repo’s file-size budget, fixes an e2e cleanup fixture edge case, and synchronizes the related docs/plugin surfaces for the 0.87.0 release.

Changes:

  • Add --workspace/--no-workspace to kbagent data-app create (default ON) and propagate the flag through service + REST.
  • Split pure request-body/redaction helpers out of data_app_service.py into services/_data_app_bodies.py.
  • Add/adjust tests (service, CLI, REST router calls, e2e) and update docs/changelog/version sync for 0.87.0.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
uv.lock Bumps locked project version to 0.87.0.
pyproject.toml Bumps package version to 0.87.0.
src/keboola_agent_cli/services/data_app_service.py Adds workspace default-on behavior and uses extracted body helpers.
src/keboola_agent_cli/services/_data_app_bodies.py New module for pure body builders + redaction helpers (incl. runtime workspace block).
src/keboola_agent_cli/commands/data_app.py Adds --workspace/--no-workspace flag and surfaces Storage-access status in human output.
src/keboola_agent_cli/server/routers/data_apps.py REST create payload gains workspace default-on and passes through to the service.
tests/test_data_app_service.py Adds service-level coverage for default workspace behavior, opt-out omission, and dry-run preview.
tests/test_data_app_cli.py Adds CLI-level coverage for flag pass-through and human output messaging.
tests/test_server_router_calls.py Adds REST router tests verifying default workspace=True and explicit disable behavior.
tests/test_e2e.py Fixes cleanup fixture robustness and adds e2e coverage for live config shape + dry-run opt-out.
src/keboola_agent_cli/commands/context.py Updates generated command context docs to include the new flag and guidance.
src/keboola_agent_cli/changelog.py Adds 0.87.0 changelog entry documenting default Storage access behavior.
scripts/file_size_baseline.json Updates LOC baseline after splitting data_app_service.py.
plugins/kbagent/skills/kbagent/references/gotchas.md Documents the “missing runtime.workspace.enabled” silent failure mode and retrofit.
plugins/kbagent/skills/kbagent/references/data-app-workflow.md Adds dedicated Storage access section and updates footgun inventory/recipes.
plugins/kbagent/skills/kbagent/references/commands-reference.md Updates data-app create signature and behavior notes.
plugins/kbagent/agents/keboola-expert.md Updates guidance matrix/triggers for 0.87.0 default Storage access behavior.
plugins/kbagent/.claude-plugin/plugin.json Syncs plugin version to 0.87.0.
.claude-plugin/marketplace.json Syncs marketplace plugin version to 0.87.0.
docs/TUTORIAL.md Updates tutorial narrative to reflect default workspace-enabled runtime block and retrofit instructions.
CLAUDE.md Updates the “All CLI Commands” section and notes to include --workspace/--no-workspace.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/keboola_agent_cli/services/_data_app_bodies.py Outdated

@keboola-pr-reviewer-bot keboola-pr-reviewer-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: needs_human (risk 3/5) · profile keboola-mcp-server

Escalating: data-app create gains a default-ON --workspace that changes default behaviour with no feature toggle to disable it without a code revert.

Impact flags: possible rollback re-introduction — see Check Run summary.

Suggested reviewers: @keboola/ai-swimlane-kai-assistant

MiroCillik and others added 3 commits August 21, 2026 15:00
The extraction cut on a line boundary that fell between a comment block
and the constant it documented, in both directions: the MAX_* caps
comment landed in _data_app_bodies.py (which has no MAX_* constant) while
the 16-line ENCRYPTED_PASSWORD_PREFIXES cipher-registry block stayed in
data_app_service.py, where it ended up above MAX_NAME_LENGTH describing
nothing. Both now sit with their own constant. No code change.

Reported by Copilot on #626.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
keboola/cli is public, so a Linear key is unopenable for any outside
reader. The reasoning was already spelled out at every one of these sites,
so the ticket token was carrying no information the prose did not. The
changelog entry keeps a reference in the repo's own idiom -- the GitHub
number (#626) -- matching the existing "Fix (#607)" / "New (#594)" style.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four fixes from a review pass over the branch.

1. RESERVED_RUNTIME_ENV_VARS gains WORKSPACE_ID, QUERY_SERVICE_URL and
   KBC_WORKSPACE_MANIFEST_PATH. Enabling the workspace by default made
   these platform-injected in the DEFAULT case, so a secret whose derived
   env-var name collides is now silently shadowed on a normal app rather
   than an exotic one -- someone migrating off the older
   parameters.dataApp.secrets.WORKSPACE_ID convention would set
   `#WORKSPACE_ID` and get no warning that the platform value wins. The
   WARN is advisory, so a false positive costs a line of stderr while a
   false negative costs a secret that silently does nothing.

2. Stop presenting an app-emitted log line as a platform diagnostic.
   `Missing env vars: WORKSPACE_ID` comes from the app's own startup check
   (the wording is the ai-kit DuckDB template's), not from the platform,
   which reports nothing at all. Written up as "the only diagnostic" it
   invited the opposite error: grep `data-app logs` on an app built from
   anything else, find nothing, conclude the flag is not the cause. All
   nine surfaces now name `config detail` -> `configuration.runtime` as the
   authoritative check and say an empty log grep rules nothing out.

3. `data-app create` reports Storage access as "requested", not "enabled".
   kbagent writes the config key; whether the runtime honours it is the
   platform's call, and the two are not the same claim.

4. Move the tutorial's retrofit recipe out of the §9.1 golden path into
   its own subsection. Sitting between the create and deploy steps, a
   reader following the walkthrough sequentially would run it, bump Storage
   to v4, and then be told by the next block that deploy pins
   configVersion=3.

RESERVED_RUNTIME_ENV_VARS and _derive_runtime_env_var_name move to
_data_app_bodies.py: (1) pushed data_app_service.py three code lines past
its grandfathered ceiling, and the budget says shrink rather than extend.
Baseline ratcheted 1648 -> 1641. keboola-expert.md's new trigger is cut to
the one-line form its own section contract requires, keeping the file under
its 62 kB prompt budget.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MiroCillik
MiroCillik requested a review from Matovidlo August 21, 2026 13:29
@linear-code

linear-code Bot commented Aug 21, 2026

Copy link
Copy Markdown

AJDA-3183

Comment thread CLAUDE.md
@MiroCillik
MiroCillik requested a review from Matovidlo August 21, 2026 14:21
@MiroCillik
MiroCillik merged commit cc2f7da into main Aug 21, 2026
4 checks passed
@MiroCillik
MiroCillik deleted the miro-ai-3753-data-app-create-workspace-flag branch August 21, 2026 15:43
claude Bot pushed a commit that referenced this pull request Aug 21, 2026
v0.87.0 was tagged and released from main (PR #626, the `data-app create`
--workspace flag), so this branch's own 0.87.0 changelog entry collided with
a published, immutable release. Resolved by moving the ai-kit publication
entry to 0.88.0 and bumping pyproject.toml accordingly; main's 0.87.0 entry
is kept verbatim.

Conflict: src/keboola_agent_cli/changelog.py -- both sides added a "0.87.0"
key. Kept both sets of bullets, under the versions the repo's one-entry-per-
release convention implies.

CONTRIBUTING.md's "planned removal ~3 releases after 0.87.0" note follows the
shim's actual ship version to 0.88.0. plugin.json / marketplace.json / uv.lock
regenerated with `make version-sync`; SKILL.md re-checked with `make skill-gen`
(no change -- the command tree did not move).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RiYtKRNF8XDx3dDZAAeBGV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants