Skip to content

Fix Snowflake workspace login type#351

Merged
padak merged 4 commits into
mainfrom
codex-snowflake-workspace-person-keypair-login-type
May 28, 2026
Merged

Fix Snowflake workspace login type#351
padak merged 4 commits into
mainfrom
codex-snowflake-workspace-person-keypair-login-type

Conversation

@zajca
Copy link
Copy Markdown
Member

@zajca zajca commented May 27, 2026

What changed

  • New config-tied Snowflake workspaces now request loginType: snowflake-person-keypair.
  • BigQuery workspaces omit loginType, preserving the Storage API default behavior.
  • The client helper now supports an optional login_type argument and only serializes it when set.
  • snowflake-person-keypair is treated as Query Service compatible in workspace list/detail enrichment.

Why

Snowflake sandbox/workspace creation was relying on the Storage API default login type. That default should only remain for BigQuery; Snowflake needs the person keypair login type for newly created workspaces.

Verification

  • env UV_CACHE_DIR=/tmp/uv-cache uv run pytest tests/test_workspace_service.py tests/test_client.py
  • env UV_CACHE_DIR=/tmp/uv-cache uv run ruff check src/keboola_agent_cli/client.py src/keboola_agent_cli/constants.py src/keboola_agent_cli/services/workspace_service.py tests/test_client.py tests/test_workspace_service.py

@zajca zajca marked this pull request as ready for review May 28, 2026 13:45
@zajca zajca requested a review from padak May 28, 2026 13:45
@zajca zajca changed the title [codex] Fix Snowflake workspace login type Fix Snowflake workspace login type May 28, 2026
Copy link
Copy Markdown
Member

@padak padak left a comment

Choose a reason for hiding this comment

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

Review of #351 — Fix Snowflake workspace login type

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check where possible, but
see the verification log for a known workaround-blocker.

Summary

This PR fixes Snowflake sandbox workspace creation by requesting loginType: snowflake-person-keypair from the Storage API, generating an RSA key pair locally, passing the public key to the API, and surfacing the private key to the user in both JSON and human-mode output. BigQuery workspaces correctly omit loginType, preserving the API default. The implementation logic and service-layer tests are solid. However, the PR is missing its changelog entry for version 0.47.1 (the only existing entry describes an unrelated storage create-table --if-not-exists fix), four plugin documentation surfaces are not updated (gotchas.md, keboola-expert.md, commands-reference.md inline note, and keboola-expert.md §3 Inline Gotchas), and the gotchas.md whitelist list that AI agents read to interpret qs_compatible is stale. These are the blocking gaps.

Verdict: REQUEST CHANGES

Verdict

  • Verdict: REQUEST CHANGES
  • Blocking findings: 4
  • Non-blocking findings: 3
  • Nits: 2

Blocking findings

[B-1] src/keboola_agent_cli/changelog.py:11 — 0.47.1 changelog entry does not describe the Snowflake keypair fix

The existing "0.47.1" entry in changelog.py covers only the storage create-table --if-not-exists behavior (#350). This PR adds a second fix to 0.47.1 (the loginType: snowflake-person-keypair workspace creation and the new private_key response field), but changelog.py is not updated to mention it. Per CONTRIBUTING.md > "Releasing a new version" step 2 ("Add a changelog entry... one entry per release, no exceptions"), make changelog-check enforces completeness at CI time. In this case the key word is completeness of the human-readable record: a user running kbagent changelog after upgrading to 0.47.1 will see only the --if-not-exists note and have no indication that workspace credential behavior changed.

Fix: append a second entry to the "0.47.1" list in changelog.py describing the Snowflake workspace loginType + private_key behavior change.

[B-2] plugins/kbagent/skills/kbagent/references/gotchas.md:147snowflake-person-keypair added to QUERY_SERVICE_COMPATIBLE_LOGIN_TYPES but the gotchas.md whitelist prose is not updated

constants.py now adds snowflake-person-keypair to QUERY_SERVICE_COMPATIBLE_LOGIN_TYPES (via SNOWFLAKE_WORKSPACE_LOGIN_TYPE). The gotchas.md section at line ~147 ("Compatibility whitelist") explicitly documents only snowflake-service-keypair and snowflake-person-sso as confirmed PASS. That prose is now stale: an AI agent reading gotchas.md will see a two-item whitelist and may incorrectly report a snowflake-person-keypair workspace as "not on the confirmed-good whitelist" even though qs_compatible: true is now correct for it.

Additionally, this is a new non-obvious behavior introduced in this release: workspace create now returns a private_key field instead of (or alongside) password for Snowflake workspaces. Per CONTRIBUTING.md > "Plugin synchronization map", gotchas.md entries for behavior changes must carry a (since vX.Y.Z) tag. Neither the whitelist update nor the private_key credential change has a tagged entry.

Fix: update the whitelist block to add snowflake-person-keypair -- confirmed PASS (since 0.47.1), and add a new titled gotcha block such as ## Snowflake workspace create returns private_key, not password (since 0.47.1) documenting that headless workspace create now returns a private_key PEM string for Snowflake (never empty on success), password is still present but empty, and how to use the key to connect.

[B-3] plugins/kbagent/agents/keboola-expert.md §3 — no inline gotcha for the credential-shape change

The keboola-expert.md §3 Inline Gotchas section is not updated. AI agents that parse the workspace create JSON response will encounter private_key in the envelope and may not know how to handle it (e.g. they might try to set up a password-based connection or warn the user the workspace has no password). This is exactly the class of non-obvious behavior §3 is intended to capture. The entry should describe that for Snowflake workspaces, the JSON envelope carries private_key (a PEM PKCS8 string) and password will be empty, so connection scripts must use the key-pair authentication path.

Per CONTRIBUTING.md > "Plugin synchronization map": keboola-expert.md has no CI check and is the highest silent-drift risk in the repo.

Fix: add a gotcha entry to §3 of plugins/kbagent/agents/keboola-expert.md describing the private_key credential shape for Snowflake workspaces.

[B-4] tests/test_e2e.py:6195 — E2E test does not assert private_key presence for Snowflake workspace create

test_issue_304_discoverability_roundtrip at line ~6195 calls workspace create but asserts only login_type, read_only, and qs_compatible on the list result. It does not assert that the data.private_key field is present in the creation response when the stack is Snowflake. Per CONTRIBUTING.md > "Tests (mandatory!)" and CLAUDE.md convention #16, every new CLI command behavior must have E2E coverage. This behavior change (headless Snowflake workspace create now returns private_key) is not covered by any E2E assertion, so a Stack API regression that silently omits the private key would not be caught.

Fix: in test_issue_304_discoverability_roundtrip (or a new test in TestE2E_0_47_0_NewSurfaces), assert "private_key" in data when the backend is snowflake and data["private_key"].startswith("-----BEGIN PRIVATE KEY-----"). Guard with pytest.skip if the stack is BigQuery.

Non-blocking findings

[NB-1] plugins/kbagent/skills/kbagent/references/commands-reference.md:131workspace create reference entry not updated for private_key

The commands-reference.md bullet for workspace create at line 131 reads -- create workspace (headless ~1s, --ui ~15s) with no mention that headless Snowflake workspaces now return private_key instead of (alongside empty) password. An AI agent reading the reference to understand what fields to expect from workspace create will not know the credential shape changed.

Fix: extend the bullet to note Since v0.47.1: Snowflake headless workspaces return a private_key PEM field; password is empty. BigQuery workspaces still return password.

[NB-2] src/keboola_agent_cli/services/workspace_service.py:300--ui mode (Queue-job path) not adapted for keypair

_create_workspace_via_job (the workspace create --ui path) does not generate or pass a keypair. After wait_for_queue_job, it calls reset_workspace_password and returns a password field. If the Keboola backend creates the workspace with snowflake-person-keypair login type by default (server-side policy, not just our explicit request), the password reset call may fail or return an unusable credential. This is not confirmed behavior -- it depends on whether the backend applies the same login type when the job path is used -- but the asymmetry between the headless and job paths is undocumented and potentially confusing.

The PR description notes "New config-tied Snowflake workspaces now request loginType: snowflake-person-keypair" without distinguishing headless vs UI mode. If this is intentionally scoped to headless only, a comment in _create_workspace_via_job explaining the behavior difference would prevent future confusion.

[NB-3] tests/test_workspace_cli.py:101test_workspace_create_success_json does not assert absence or presence of private_key in JSON output

The existing JSON-mode CLI test at line 101 mocks create_workspace to return a dict without private_key, asserting only password and backend. Since the real service now always adds private_key for Snowflake, a consumer parsing workspace create --json output needs to branch on whether private_key is present. The test does not cover the JSON-mode Snowflake case with private_key populated, so the shape is only verified in human mode (by test_workspace_create_human_outputs_private_key_when_present). Adding a JSON-mode test where the mock returns private_key would verify the key survives formatter.output to the CLI consumer.

Nits

  • [NIT-1] src/keboola_agent_cli/services/workspace_service.py:42_generate_snowflake_workspace_key_pair returns tuple[str, str] with positionally-distinct values (private key, public key). Per CONTRIBUTING.md Code Quality Patterns, two-element tuples whose values are semantically distinct should use a @dataclass. A SnowflakeKeyPair(private_pem: str, public_pem: str) dataclass would make callers self-documenting and eliminate the positional swap risk at call sites. The existing grandfathered tuple[...] returns in services are noted as migration candidates; this is new code, so the pattern applies.

  • [NIT-2] src/keboola_agent_cli/hints/definitions/workspace.py:16 — the ClientCall for workspace.create specifies args={"backend": "{backend}"} without documenting the login_type and public_key parameters that create_config_workspace now accepts. A caller generating a hint snippet would get a function call that omits these new mandatory-for-Snowflake parameters. Since the hint is a code-generation surface, updating the ClientCall args to include login_type and public_key (even as comment placeholders) would make the generated code usable.

Verification log

  • gh auth status → authenticated to github.com as padak ✓
  • gh pr view 351 --repo keboola/cli --json ... → state: OPEN, +315/-23, conventional commit title "Fix Snowflake workspace login type" (no conventional-commit prefix in PR title -- informational only, title is not the commit message)
  • gh pr diff 351 --repo keboola/cli → 637 lines, 9 files: pyproject.toml, client.py, commands/workspace.py, constants.py, services/workspace_service.py, tests/test_client.py, tests/test_workspace_cli.py, tests/test_workspace_service.py, uv.lock
  • git rev-parse --abbrev-ref HEADclaude/cranky-hodgkin-c3bc96 (NOT on PR branch); all source reads done via gh api repos/keboola/cli/contents/<path>?ref=33c1c4f
  • Layer violation greps (typer in services, httpx in commands, formatter in clients) → all empty ✓
  • Bare except: grep → empty ✓
  • print() in production code grep → empty ✓
  • Magic-number grep (timeout/retries/sleep) → empty ✓
  • Raw error_code="..." string literals grep → empty ✓
  • permissions.py search for workspace entries → all 9 workspace commands present, no new command added in this PR, permission engine unaffected ✓
  • hints/definitions/workspace.py at PR HEAD → workspace.create hint exists; ClientCall.args still only {"backend": "{backend}"} -- omits login_type/public_key (NIT-2)
  • commands/context.py grep for workspace → signatures unchanged, no new command in this PR ✓ (not a silent-drift gap)
  • CLAUDE.md ##AllCLICommands grep → workspace signatures unchanged ✓
  • permissions.py → no new operation, no gap ✓
  • changelog.py "0.47.1" at PR HEAD → contains only the storage create-table --if-not-exists entry, keypair fix absent → B-1
  • gotchas.md grep for snowflake-person-keypair → empty; whitelist section still shows only 2-item list → B-2
  • gotchas.md grep for private.key\|private_key → empty; no new gotcha entry with (since v0.47.1) → B-2
  • keboola-expert.md grep for keypair\|private.key\|workspace.*create.*key → empty → B-3
  • test_e2e.py grep for private_key\|snowflake.person → empty in E2E file → B-4
  • cryptography in pyproject.toml on main → absent (was only transitive dep); now promoted to direct dep cryptography>=46 in this PR ✓ (correct, the package now directly uses cryptography.hazmat.primitives)
  • _create_workspace_via_job at PR HEAD → no keypair generation, returns password only, message still says "Save the password" → NB-2
  • test_workspace_create_success_json at PR HEAD → mocks service without private_key, asserts only password and backend; JSON-mode private_key shape uncovered → NB-3
  • make check → BLOCKED by auto-mode classifier (worktree is on branch claude/cranky-hodgkin-c3bc96, not on PR branch; running the full test suite on a stale branch was denied). PR description states the author ran uv run pytest tests/test_workspace_service.py tests/test_client.py plus ruff check on the five touched source files -- treating as passing for those files. Full suite status unverified here.

Open questions for the author

  1. Does the real Keboola Storage API return BOTH password AND the keypair credentials for snowflake-person-keypair workspaces, or only the keypair? The test fixture SAMPLE_WORKSPACE still includes "password": "s3cret!Passw0rd" in connection, and test_create_workspace_success asserts result["password"] == "s3cret!Passw0rd" alongside result["private_key"]. If the real API omits password for keypair workspaces, the test fixture is misleading (though not functionally broken since the service falls back to connection.get("password", "")). Confirming the API shape in the PR description would help reviewers evaluate correctness without needing E2E access.

  2. Is --ui mode (workspace create --ui) intentionally excluded from the keypair login type? The PR description says "New config-tied Snowflake workspaces now request loginType: snowflake-person-keypair" but _create_workspace_via_job is unchanged and will continue to create workspaces via the Sandboxes job without requesting the keypair type. If the backend enforces keypair by policy regardless of the path, the reset_workspace_password call in the UI path may fail or be a no-op. If it is intentionally excluded, the PR description and a code comment would prevent future confusion.

Copy link
Copy Markdown
Member

@padak padak left a comment

Choose a reason for hiding this comment

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

Review of #351 — Fix Snowflake workspace login type (re-review after fix commit 8423f76)

Generated by kbagent-pr-reviewer subagent. This is a re-review following the prior
REQUEST CHANGES verdict (4 blocking, 3 non-blocking, 2 nits). The fix commit
8423f76b was targeted at those findings. This review verifies each prior finding
independently against the actual file content at 8423f76b, then checks for
regressions in the service-layer refactor.

Prior findings status — full delta

Finding Prior verdict Resolution status
B-1: changelog.py must describe the keypair fix BLOCKING RESOLVED
B-2: gotchas.md — snowflake-person-keypair in whitelist + versioned private_key section BLOCKING RESOLVED
B-3: keboola-expert.md §3 — Snowflake private_key inline gotcha BLOCKING RESOLVED
B-4: test_e2e.py — private_key assertion on Snowflake stack BLOCKING RESOLVED
N-1: --ui Queue-job path asymmetry documented NON-BLOCKING RESOLVED (comment added)
N-2: JSON-mode CLI test for private_key NON-BLOCKING RESOLVED
N-3: commands-reference.md stale description NON-BLOCKING RESOLVED

Summary

The fix commit 8423f76b addresses all four prior BLOCKING findings and all three
NON-BLOCKING findings. Every resolution was verified against the actual file content
at the fix commit HEAD, not just the file list. The service-layer refactor (the
tuple[str, str] -> SnowflakeWorkspaceKeyPair dataclass conversion, 26+/14-) is a
clean improvement that follows the CONTRIBUTING.md Code Quality Patterns guideline
("multi-value returns use a @dataclass, never a bare tuple") and introduces no
behavior regression on the password path.

One new NON-BLOCKING finding surfaced during the review: the --hint client definition
for workspace create uses bare string literals "login_type" and "public_key_pem"
as ClientCall args instead of the {placeholder} convention or a quoted Python
literal. The generated code would emit login_type=login_type (a Python name
reference) rather than a usable default. This is a documentation-quality issue, not
a runtime regression; the service-layer hint (--hint service) is unaffected and
correct. One NIT was also identified regarding the keboola-expert.md description
shortening.

Verdict: APPROVE — all prior blocking issues are genuinely resolved, make check
passes clean (3617 passed, 7 skipped), and the one new finding is non-blocking.

Verdict

  • Verdict: APPROVE
  • Blocking findings: 0
  • Non-blocking findings: 1 (new)
  • Nits: 1 (new)

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/hints/definitions/workspace.py:22-23--hint client workspace create emits unresolvable Python name references

The fix commit adds "login_type": "login_type" and "public_key": "public_key_pem"
to the ClientCall.args dict. The hints renderer (renderer.py:_substitute_params)
passes values without {...} placeholders through as-is, then formats them as
login_type=login_type and public_key=public_key_pem in the generated code.
These are bare Python name references that will cause a NameError for any caller
who runs the generated snippet as-is. Other hints use either {placeholder} (for
CLI parameter substitution) or a quoted literal like '"linkedBuckets"' (for
fixed string values). The correct forms here are either '"snowflake-person-keypair"'
(fixed string) or "{login_type}" (CLI param, which does not exist on workspace create, so not valid either). The --hint service path is unaffected because
ServiceCall.args only carries the CLI flags, not the internal login_type/public_key
details. Fix: change the args to '"snowflake-person-keypair"' and "public_key_pem" with a
comment explaining that callers must generate the key pair and substitute
public_key_pem, or remove login_type and public_key from ClientCall.args
and expand the existing notes entry (which already says to generate the key pair)
into a more explicit snippet.

Nits

  • [NIT-1] plugins/kbagent/agents/keboola-expert.md:3 — The fix commit shortened the description: field from 87 words (which enumerated the task types that trigger the subagent: "config browsing/updates, jobs, flows, schedules, storage, migrations, dev branches, debugging") to 16 words. The description is the trigger surface Claude Code uses for auto-invocation of the subagent; less-specific descriptions may reduce proactive delegation. This was presumably intentional (Claude description-matching has a token budget), but worth an explicit confirmation from the author that the shorter form still triggers reliably for the relevant task types.

Verification log

  • gh pr view 351 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → state=OPEN, 16 files, +386/-27, branch codex-snowflake-workspace-person-keypair-login-type vs main
  • gh auth status → authenticated as padak on github.com
  • CONTRIBUTING.md Plugin synchronization map — read ✓; CLAUDE.md §17 silent-drift surfaces — read ✓; plugins/kbagent/agents/keboola-expert.md §1 + §3 — read ✓
  • gh pr diff 351 > /tmp/kbagent-pr-351.diff → 842 lines ✓
  • B-1 (changelog.py): gh api repos/keboola/cli/contents/src/keboola_agent_cli/changelog.py?ref=8423f76b..."0.47.1" key contains two entries: one for --if-not-exists (PR #349) and one explicitly for workspace create keypair fix (PR #351). RESOLVED ✓
  • B-2 (gotchas.md): file at 8423f76b line 151 has snowflake-person-keypair -- confirmed PASS (since v0.47.1) in the whitelist. New section ## Snowflake workspace create returns private_key, not password (since v0.47.1) added at line 163 with example JSON envelope. Both requirements satisfied. RESOLVED ✓
  • B-3 (keboola-expert.md §3): §3 at 8423f76b lines 223-228 has **Snowflake workspace credentials** (0.47.1+) gotcha with private_key / empty password guidance. Rule 6 VERSION GATE updated at line 114 with Snowflake workspace create private_key = 0.47.1+. RESOLVED ✓
  • B-4 (test_e2e.py): test_snowflake_workspace_create_returns_private_key() added; asserts "private_key" in data and data["private_key"].startswith("-----BEGIN PRIVATE KEY-----"); skips gracefully on non-Snowflake stacks. RESOLVED ✓
  • N-1 (Queue-job path asymmetry): _create_workspace_via_job() in fix commit gains comment: "The Queue job path does not expose a publicKey/loginType input. Keep returning a reset password here; headless Snowflake creates use the key-pair path in _create_workspace_direct()." Asymmetry is now explicitly documented at the code site. RESOLVED ✓
  • N-2 (JSON-mode CLI test): test_workspace_create_success_json in test_workspace_cli.py now asserts output["data"]["private_key"].startswith("-----BEGIN PRIVATE KEY-----") (diff line 552). New test_workspace_create_human_outputs_private_key_when_present tests human-mode output separately. RESOLVED ✓
  • N-3 (commands-reference.md): workspace create entry updated: "Since v0.47.1: Snowflake headless workspaces return a private_key PEM field; password is empty. BigQuery workspaces keep the default password credential shape." RESOLVED ✓
  • Service-layer refactor regression check: _workspace_key_pair_for_backend() changed from returning tuple[str|None, str|None] to SnowflakeWorkspaceKeyPair | None. Both call sites (_create_workspace_direct, create_from_transformation) updated symmetrically. BigQuery path: key_pair=None, public_key=None passed to create_config_workspace, password field still populated from connection.get("password", ""), private_key key NOT added to result dict. Password path unchanged. No regression. ✓
  • Layer compliance: no typer/formatter imports in services, no httpx in commands. cryptography import in workspace_service.py is at the service layer (correct). ✓
  • Security: private_key PEM is generated locally and passed directly to the CLI output dict; it is not logged. No token exposure in new code. ✓
  • uv sync --extra server && make check3617 passed, 7 skipped, 107 deselected, 14 warnings in 85.98s
  • E2E behavior: could not reproduce (no Snowflake-stack E2E credentials in this environment); the test uses pytest.skip() on non-Snowflake stacks, so it is safe to run in any environment. Unverified at runtime — flagged as an open verification gap, not a finding.
  • Hint rendering (NB-1): --hint client workspace create on local install produces client.create_config_workspace() with no args (pre-fix version); the new hint definition was inspected at 8423f76b. _substitute_params code path confirmed: values without {...} are passed through as-is, yielding login_type=login_type and public_key=public_key_pem in generated code — unresolvable Python name references. No test covers this output. NON-BLOCKING finding raised.

Open questions for the author

  • NB-1 fix direction: should ClientCall.args for workspace create omit login_type/public_key entirely (since the service hint already captures the full semantics) and rely on the notes field to guide direct-client users? Or is there a preferred way to document "caller must supply this" in the hint system that already works for other commands?

Copy link
Copy Markdown
Member

@padak padak left a comment

Choose a reason for hiding this comment

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

Review of #351 — Fix Snowflake workspace login type (re-review after fix commit 8423f76)

Generated by kbagent-pr-reviewer subagent. This is a re-review following the prior
REQUEST CHANGES verdict (4 blocking, 3 non-blocking, 2 nits). The fix commit
8423f76b was targeted at those findings. This review verifies each prior finding
independently against the actual file content at 8423f76b, then checks for
regressions in the service-layer refactor.

Prior findings status — full delta

Finding Prior verdict Resolution status
B-1: changelog.py must describe the keypair fix BLOCKING RESOLVED
B-2: gotchas.md — snowflake-person-keypair in whitelist + versioned private_key section BLOCKING RESOLVED
B-3: keboola-expert.md §3 — Snowflake private_key inline gotcha BLOCKING RESOLVED
B-4: test_e2e.py — private_key assertion on Snowflake stack BLOCKING RESOLVED
N-1: --ui Queue-job path asymmetry documented NON-BLOCKING RESOLVED (comment added)
N-2: JSON-mode CLI test for private_key NON-BLOCKING RESOLVED
N-3: commands-reference.md stale description NON-BLOCKING RESOLVED

Summary

The fix commit 8423f76b addresses all four prior BLOCKING findings and all three
NON-BLOCKING findings. Every resolution was verified against the actual file content
at the fix commit HEAD, not just the file list. The service-layer refactor (the
tuple[str, str] -> SnowflakeWorkspaceKeyPair dataclass conversion, 26+/14-) is a
clean improvement that follows the CONTRIBUTING.md Code Quality Patterns guideline
("multi-value returns use a @dataclass, never a bare tuple") and introduces no
behavior regression on the password path.

One new NON-BLOCKING finding surfaced during the review: the --hint client definition
for workspace create uses bare string literals "login_type" and "public_key_pem"
as ClientCall args instead of the {placeholder} convention or a quoted Python
literal. The generated code would emit login_type=login_type (a Python name
reference) rather than a usable default. This is a documentation-quality issue, not
a runtime regression; the service-layer hint (--hint service) is unaffected and
correct. One NIT was also identified regarding the keboola-expert.md description
shortening.

Verdict: APPROVE — all prior blocking issues are genuinely resolved, make check
passes clean (3617 passed, 7 skipped), and the one new finding is non-blocking.

Verdict

  • Verdict: APPROVE
  • Blocking findings: 0
  • Non-blocking findings: 1 (new)
  • Nits: 1 (new)

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/hints/definitions/workspace.py:22-23--hint client workspace create emits unresolvable Python name references

The fix commit adds "login_type": "login_type" and "public_key": "public_key_pem"
to the ClientCall.args dict. The hints renderer (renderer.py:_substitute_params)
passes values without {...} placeholders through as-is, then formats them as
login_type=login_type and public_key=public_key_pem in the generated code.
These are bare Python name references that will cause a NameError for any caller
who runs the generated snippet as-is. Other hints use either {placeholder} (for
CLI parameter substitution) or a quoted literal like '"linkedBuckets"' (for
fixed string values). The correct forms here are either '"snowflake-person-keypair"'
(fixed string) or "{login_type}" (CLI param, which does not exist on workspace create, so not valid either). The --hint service path is unaffected because
ServiceCall.args only carries the CLI flags, not the internal login_type/public_key
details. Fix: change the args to '"snowflake-person-keypair"' and "public_key_pem" with a
comment explaining that callers must generate the key pair and substitute
public_key_pem, or remove login_type and public_key from ClientCall.args
and expand the existing notes entry (which already says to generate the key pair)
into a more explicit snippet.

Nits

  • [NIT-1] plugins/kbagent/agents/keboola-expert.md:3 — The fix commit shortened the description: field from 87 words (which enumerated the task types that trigger the subagent: "config browsing/updates, jobs, flows, schedules, storage, migrations, dev branches, debugging") to 16 words. The description is the trigger surface Claude Code uses for auto-invocation of the subagent; less-specific descriptions may reduce proactive delegation. This was presumably intentional (Claude description-matching has a token budget), but worth an explicit confirmation from the author that the shorter form still triggers reliably for the relevant task types.

Verification log

  • gh pr view 351 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → state=OPEN, 16 files, +386/-27, branch codex-snowflake-workspace-person-keypair-login-type vs main
  • gh auth status → authenticated as padak on github.com
  • CONTRIBUTING.md Plugin synchronization map — read ✓; CLAUDE.md §17 silent-drift surfaces — read ✓; plugins/kbagent/agents/keboola-expert.md §1 + §3 — read ✓
  • gh pr diff 351 > /tmp/kbagent-pr-351.diff → 842 lines ✓
  • B-1 (changelog.py): gh api repos/keboola/cli/contents/src/keboola_agent_cli/changelog.py?ref=8423f76b..."0.47.1" key contains two entries: one for --if-not-exists (PR #349) and one explicitly for workspace create keypair fix (PR #351). RESOLVED ✓
  • B-2 (gotchas.md): file at 8423f76b line 151 has snowflake-person-keypair -- confirmed PASS (since v0.47.1) in the whitelist. New section ## Snowflake workspace create returns private_key, not password (since v0.47.1) added at line 163 with example JSON envelope. Both requirements satisfied. RESOLVED ✓
  • B-3 (keboola-expert.md §3): §3 at 8423f76b lines 223-228 has **Snowflake workspace credentials** (0.47.1+) gotcha with private_key / empty password guidance. Rule 6 VERSION GATE updated at line 114 with Snowflake workspace create private_key = 0.47.1+. RESOLVED ✓
  • B-4 (test_e2e.py): test_snowflake_workspace_create_returns_private_key() added; asserts "private_key" in data and data["private_key"].startswith("-----BEGIN PRIVATE KEY-----"); skips gracefully on non-Snowflake stacks. RESOLVED ✓
  • N-1 (Queue-job path asymmetry): _create_workspace_via_job() in fix commit gains comment: "The Queue job path does not expose a publicKey/loginType input. Keep returning a reset password here; headless Snowflake creates use the key-pair path in _create_workspace_direct()." Asymmetry is now explicitly documented at the code site. RESOLVED ✓
  • N-2 (JSON-mode CLI test): test_workspace_create_success_json in test_workspace_cli.py now asserts output["data"]["private_key"].startswith("-----BEGIN PRIVATE KEY-----") (diff line 552). New test_workspace_create_human_outputs_private_key_when_present tests human-mode output separately. RESOLVED ✓
  • N-3 (commands-reference.md): workspace create entry updated: "Since v0.47.1: Snowflake headless workspaces return a private_key PEM field; password is empty. BigQuery workspaces keep the default password credential shape." RESOLVED ✓
  • Service-layer refactor regression check: _workspace_key_pair_for_backend() changed from returning tuple[str|None, str|None] to SnowflakeWorkspaceKeyPair | None. Both call sites (_create_workspace_direct, create_from_transformation) updated symmetrically. BigQuery path: key_pair=None, public_key=None passed to create_config_workspace, password field still populated from connection.get("password", ""), private_key key NOT added to result dict. Password path unchanged. No regression. ✓
  • Layer compliance: no typer/formatter imports in services, no httpx in commands. cryptography import in workspace_service.py is at the service layer (correct). ✓
  • Security: private_key PEM is generated locally and passed directly to the CLI output dict; it is not logged. No token exposure in new code. ✓
  • uv sync --extra server && make check3617 passed, 7 skipped, 107 deselected, 14 warnings in 85.98s
  • E2E behavior: could not reproduce (no Snowflake-stack E2E credentials in this environment); the test uses pytest.skip() on non-Snowflake stacks, so it is safe to run in any environment. Unverified at runtime — flagged as an open verification gap, not a finding.
  • Hint rendering (NB-1): --hint client workspace create on local install produces client.create_config_workspace() with no args (pre-fix version); the new hint definition was inspected at 8423f76b. _substitute_params code path confirmed: values without {...} are passed through as-is, yielding login_type=login_type and public_key=public_key_pem in generated code — unresolvable Python name references. No test covers this output. NON-BLOCKING finding raised.

Open questions for the author

  • NB-1 fix direction: should ClientCall.args for workspace create omit login_type/public_key entirely (since the service hint already captures the full semantics) and rely on the notes field to guide direct-client users? Or is there a preferred way to document "caller must supply this" in the hint system that already works for other commands?

@padak padak merged commit 9a874dc into main May 28, 2026
2 checks passed
@padak padak deleted the codex-snowflake-workspace-person-keypair-login-type branch May 28, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants