Skip to content

feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics#348

Open
ottomansky wants to merge 11 commits into
keboola:mainfrom
ottomansky:feat/fresh-create-completeness
Open

feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics#348
ottomansky wants to merge 11 commits into
keboola:mainfrom
ottomansky:feat/fresh-create-completeness

Conversation

@ottomansky
Copy link
Copy Markdown
Contributor

@ottomansky ottomansky commented May 26, 2026

Summary

Bundles three coherent additions for v0.47.0 that close FIIA → kbagent migration pain points and add MCP-parity surfaces to the CLI.

  • Area B — sync push fresh-CREATE correctness: placeholder manifest entries seeded by FIIA / scaffold tools are now updated in place (no manifest duplication, naturally idempotent re-pushes); any KBC.configuration.* keys on a placeholder are propagated to the remote via set_config_metadata immediately after the create.
  • Area A — semantic-layer reads: kbagent semantic-layer search-context + get-context — project-wide glob search and single-id lookup mirroring the upstream keboola-mcp-server search_semantic_context / get_semantic_context MCP tools, so downstream callers can drop their MCP dependency for the common pre-flight checks.
  • Area C — ergonomics: sync push|pull|diff --branch <id> (per-invocation dev-branch override), storage create-table --if-not-exists (idempotency for parallel workers), sync push --no-name-drift-warnings (cosmetic-noise opt-out).

7 commits, ~1300 net lines, full silent-drift sync-map walked, 4 new e2e tests against a real Keboola project. Convergence: independent iteration-2 and iteration-3 reviewers both returned ship-ready.

Why one PR

The three areas share two underlying surfaces (SyncService.push / pull / diff and the manifest writeback path) and a single release version. Splitting into three would mean three rounds through the same reviewer touching overlapping sync_service.py regions. The trade-off is a larger diff per review; the alternative is more reviewer round-trips, not fewer reviewer-minutes.

What this PR includes

# Area Change Severity Primary file
1 Area B Manifest writeback updates placeholder entries in place (not append) on CREATE P1 — bug services/sync_service.py
2 Area B Propagate KBC.configuration.* metadata via set_config_metadata on CREATE P2 — bug services/sync_service.py
3 Area A kbagent semantic-layer search-context subcommand P0 — gap commands/semantic_layer.py, services/semantic_layer_service.py, server/routers/semantic_layer.py
4 Area A kbagent semantic-layer get-context subcommand P0 — gap same as #3
5 Area C sync push/pull/diff --branch <id> P2 — ergonomics commands/sync.py, services/sync_service.py
6 Area C storage create-table --if-not-exists P2 — ergonomics services/storage_service.py, commands/storage.py
7 Area C sync push --no-name-drift-warnings P3 — ergonomics commands/sync.py, services/sync_service.py

Two related fixes from an earlier proposal were already shipped in v0.46.1 and dropped from scope: variable-row parent-ULID resolution (sync_service.py) and row values preservation (config_format.py ROW_HOIST_COMPONENTS).

Manifest contract change (BREAKING for direct manifest readers)

Pre-0.47.0: a single CREATE produced two manifest entries — the original placeholder + a fresh one with the assigned ULID.
Post-0.47.0: a single CREATE produces a single manifest entry — the placeholder is updated in place by (branch_id, component_id, path) and its KBC.* metadata is preserved.

Downstream consumers that parsed the manifest expecting placeholder + ULID duplicates need to drop that workaround. Documented in gotchas.md under (since v0.47.0). The Manifest.version field is unchanged at 3 (no schema diff; only the writeback flow changed).

Sync push REST exemption

kbagent sync push|pull|diff remain filesystem-local and intentionally have no HTTP router in src/keboola_agent_cli/server/routers/. The new --branch and --no-name-drift-warnings flags consequently also have no REST counterpart. The CONTRIBUTING.md plugin-sync map permits this exemption for terminal-only / filesystem-bound commands. All other new surfaces (storage create-table --if-not-exists, semantic-layer search-context, semantic-layer get-context) are exposed 1:1 over HTTP.

Tests added

Layer New tests Files
Service-layer unit 18 (search-context: 12, get-context: 6) tests/test_semantic_layer_service.py
Service-layer unit 5 (storage create-table --if-not-exists) tests/test_storage_write.py
Service-layer unit 4 (--branch override resolver, --no-name-drift-warnings suppression) tests/test_sync_service.py
Service-layer unit 9 (fresh-CREATE writeback + KBC propagation + multi-branch safety + metadata-propagation error accumulation) tests/test_sync_service.py::TestFreshCreateWriteback
CLI-layer 7 (search-context: 4, get-context: 3) tests/test_semantic_layer_cli.py
CLI-layer 1 (--if-not-exists human-mode skip render) tests/test_storage_write.py
E2E 4 (TestE2E_0_47_0_NewSurfaces: storage IF-NOT-EXISTS round-trip; semantic-layer search + get; sync diff --branch; fresh-CREATE writeback + KBC. propagation against real metadata API*) tests/test_e2e.py

make check clean: 3613 passed, 7 skipped, 106 deselected, 0 failed. ty check clean. ruff check + ruff format --check clean. make skill-check / version-check / changelog-check / check-error-codes all pass.

Live validation receipts (project 1143 / 99_Playground_Max, europe-west3.gcp)

Surface Receipt
semantic-layer search-context --pattern '*' total_count: 8, types: [constraint, dataset, metric, relationship]
semantic-layer search-context --pattern 'rev_*' --type metric total_count: 1, hit: rev_probe_a
semantic-layer get-context <uuid> resolved to full attribute dict
semantic-layer get-context 00000000-... (invalid UUID) status: error, code: NOT_FOUND, exit: 1 after probing all 6 types
sync push fresh-CREATE on dev branch 388071 created: 1, errors: 0, manifest entry updated in place at id 01ksgc45genpcb6tkjtz8qyr5a, KBC.configuration.folderName: 'Area B E2E Folder' confirmed via config metadata-list
Re-push of the same workspace status: no_changes, created: 0, errors: 0 (idempotent)
sync diff --branch 388072 reaches the dev branch, reports remote_only: 31
storage create-table --if-not-exists × 2 first: action: created; second: action: skipped, skip_reason: 'table already exists'
storage create-table × 3rd without flag original STORAGE_JOB_FAILED envelope, exit 1
test_sync_push_fresh_create_writeback_and_kbc_metadata (new e2e test) passes 9.44s end-to-end against the live API

All dev branches and buckets used for validation were deleted in teardown.

Convergence (independent review loop)

Iteration Reviewer Code findings Security findings Verdict
1 Self (during implementation) sync-map walk + tests inline
2 Fresh independent agent (no conversation context) 0 BLOCKING, 4 NON-BLOCKING, 3 NIT 0 BLOCKING, 0 NON-BLOCKING, 2 NIT All NON-BLOCKING + 1 NIT fixed in commit 372aa99
3 Fresh independent convergence reviewer 0 BLOCKING, 0 NON-BLOCKING, 3 NIT 0 BLOCKING, 0 NON-BLOCKING, 1 NIT CONVERGED — zero material findings; 2 doc NITs fixed in f398eb1, 1 follow-up NIT below

Follow-up issues to file before merge

Per the contributor's deferred-scope-orphan-prevention rule, any deferred scope must be tracked as a real GitHub issue and linked from the PR body before merge:

Test plan

  • Service-layer unit tests for every new code path (mocked client)
  • CLI-layer tests via CliRunner (human + JSON modes, error paths)
  • E2E tests in tests/test_e2e.py against a real Keboola project for every new CLI surface
  • make check green (lint + format + skill + version + changelog + error-codes + 3613 tests)
  • ty check clean (0 diagnostics)
  • Live validation against project 1143 with receipts captured above
  • Plugin sync-map walked (AGENT_CONTEXT, CLAUDE.md, keboola-expert.md, commands-reference.md, gotchas.md, sync-workflow.md; SKILL.md + plugin.json + marketplace.json regenerated)
  • Two-iteration independent review loop converged to zero material findings
  • Pre-existing flake TestFullE2E::test_full_cli_e2e::_test_file_operations — unrelated Storage Files indexing lag; not introduced by this PR

🤖 Generated with Claude Code

…pagate KBC.* metadata on create

Fresh-CREATE pre-population (FIIA / scaffold emit pattern): downstream
callers seed manifest entries with placeholder ids and (optionally)
KBC.configuration.* metadata before the first `sync push`. Pre-fix, every
create unconditionally appended a new ManifestConfiguration / ManifestConfigRow
to the manifest, so N placeholders -> 2N entries after one push, every
placeholder still looked "added" on re-push (spurious duplicates on remote),
and KBC.configuration.folderName from local manifest was silently dropped.

Changes:
- Service: new `_writeback_create_config_in_manifest` finds the placeholder
  by (component_id, path) and updates id + branch_id + pull_hash /
  pull_config_hash in place; preserves all non-bookkeeping metadata
  (KBC.*). Append remains the fallback when no placeholder exists.
- Service: matching `_writeback_create_row_in_manifest` for rows under a
  parent config.
- Service: new `_propagate_kbc_metadata` POSTs any KBC.* keys from the
  manifest entry to `client.set_config_metadata` once, immediately after
  the create call. Bookkeeping keys (pull_hash, pull_config_hash) stay
  out of the metadata API.
- push_changes(): replaces the inline `manifest.configurations.append(...)`
  block (config create path) with the helper call + propagation.
- _push_create_row(): replaces `parent.rows.append(...)` with the row
  helper.

Idempotency on re-push falls out for free: after the first push, the
placeholder entry holds the real ULID, so the diff engine finds it in
remote_configs and reports no change.

Tests (TestFreshCreateWriteback, 7 cases):
- writeback config in place (placeholder + KBC.* metadata preserved)
- writeback config falls back to append when no placeholder
- propagate_kbc_metadata filters bookkeeping keys, calls set_config_metadata
- propagate_kbc_metadata no-op when there are no KBC.* keys
- writeback row in place (no manifest growth)
- writeback row falls back to append for untracked rows
- end-to-end push: placeholder + folderName -> create + set_config_metadata,
  manifest length unchanged, re-push is a no-op (status=no_changes)

Full sync test suite (77 cases in test_sync_service.py) green; full repo
suite (3576 passed, 110 skipped) green; `ty check` clean.
…de reads

Two project-wide read subcommands that mirror the upstream
`keboola-mcp-server` semantic-context tools (`search_semantic_context`,
`get_semantic_context`). Lets downstream callers (FIIA, scheduled agents)
drop their MCP dependency for the common "is the model populated?" and
"what's at this id?" lookups.

CLI:
- `kbagent semantic-layer search-context --project P [--pattern G ...]
  [--type model|dataset|metric|relationship|constraint|glossary|all]
  [--limit N]` — project-wide glob search over entity names; default
  searches every child type (not the model itself); `*` matches all.
  Patterns are repeatable, taking the union. Case-sensitive fnmatch.
  `--limit` short-circuits both inner and outer loops.
- `kbagent semantic-layer get-context --project P --context-id ID` —
  single fetch by id; probes semantic-model + every CHILD_TYPES entry
  until it hits, raises NOT_FOUND if no type matches. Non-404 errors
  (500, etc.) propagate immediately rather than being swallowed.

Service (`SemanticLayerService.search_context` /
`SemanticLayerService.get_context`):
- Validation at the service boundary so CLI, REST router, and
  `--hint service` callers all share the same error shape.
- `_strip_semantic_prefix` normalises the response type field from
  `"semantic-dataset"` to `"dataset"` for the CLI surface.
- Lookup order in get_context is model-first so a model hit short-
  circuits the 6-type probe to a single call.
- try/finally guarantees the metastore client is closed on success and
  on every error path.

Sync surfaces touched:
- `commands/semantic_layer.py` — two new Typer commands with
  `should_hint`/`emit_hint` short-circuits per the hint convention.
- `services/semantic_layer_service.py` — new methods + new ClassVar
  `_ALL_TYPES_FOR_LOOKUP` tuple.
- `server/routers/semantic_layer.py` — `GET /search-context` and
  `GET /get-context` (1:1 CLI->HTTP per CONTRIBUTING.md plugin-sync map);
  `Query` added to fastapi import.
- `hints/definitions/semantic_layer.py` — two new `CommandHint` entries
  with `ClientCall` + `ServiceCall` for `--hint client` /
  `--hint service` code generation.
- `permissions.py` — both registered as `read` operations.

Tests:
- `tests/test_semantic_layer_service.py::TestSearchContext` (12 cases) —
  default pattern, glob narrowing, case-sensitivity, multi-pattern union,
  type filter (singular + `all` + `model`), `--limit` short-circuit,
  invalid type / empty pattern / zero limit validation, client cleanup
  on API error.
- `tests/test_semantic_layer_service.py::TestGetContext` (6 cases) —
  finds dataset by id, finds model (short-circuit on first probe),
  NOT_FOUND after exhausting all 6 types, 500 propagates without
  swallowing, empty-id validation, client cleanup on error.
- `tests/test_semantic_layer_cli.py::TestSearchContext` (4 cases) and
  `::TestGetContext` (3 cases) — JSON envelope, kwarg propagation,
  human-mode table rendering, NOT_FOUND non-zero exit code.

Live validation against project 1143 (99_Playground_Max):
- `search-context --pattern "*"` returns 8 contexts spanning 4 types.
- `search-context --pattern "rev_*" --type metric` narrows to 1 hit.
- `get-context` with a UUID returned by the search resolves to its
  full attribute dict.
- `get-context` with `00000000-0000-0000-0000-000000000000` returns
  NOT_FOUND envelope (exit 1) after probing all 6 types.

Full test suite (3601 passed, 110 skipped) green; `ty check` clean.
…-drift-warnings

Three ergonomic improvements that close downstream-tooling pain points
encountered during the FIIA -> kbagent migration.

`kbagent sync push --branch <id>` (also sync pull / sync diff):
- Per-invocation dev-branch targeting. Beats manifest.branches[0],
  active_branch_id, and branch-mapping.json (priority 0 in the resolver).
- Lets a downstream caller (or operator) target a freshly-created dev
  branch without first running `branch use` or `sync branch-link`.
- Validated mutually exclusive with --all-projects at the CLI layer.
- Symmetric on pull / diff for predictable UX.
- Threaded through `SyncService._resolve_branch_id(..., branch_override=)`.

`kbagent storage create-table --if-not-exists`:
- Opt-in flag (defaults False so existing callers are unaffected).
- When set, catches the specific `STORAGE_JOB_FAILED` + "already has the
  same display name" error, probes `get_table_detail(target_id)` to
  confirm the table truly exists at the expected id, and returns
  `{action: "skipped", skip_reason: "table already exists"}` instead of
  raising. A different table with the same display name still surfaces
  the original error (a real conflict to resolve).
- Solves the FIIA `scaffold_storage.py` 8-worker spurious-error symptom
  documented in the original proposal.

`kbagent sync push --no-name-drift-warnings`:
- Opt-out flag to suppress the cosmetic `name_drift_warnings` array in
  the result envelope. The underlying detection still runs (so a future
  reviewer can re-enable it); only the report is dropped.

Sync surfaces touched:
- `services/sync_service.py` -- `_resolve_branch_id` gains a
  `branch_override` parameter (priority 0); `push` / `pull` / `diff`
  thread it through. `push` adds `no_name_drift_warnings` flag with a
  single-line suppression at the result-envelope step.
- `services/storage_service.py` -- `create_table` gains
  `if_not_exists=False` kwarg; the IF-NOT-EXISTS branch wraps the
  existing client.create_table call with a targeted try/except that
  uses `ErrorCode.STORAGE_JOB_FAILED` (no raw string literal).
  Response envelope now carries `action: "created" | "skipped"` so
  programmatic callers can branch on outcome.
- `commands/sync.py` -- adds `--branch` to push / pull / diff;
  adds `--no-name-drift-warnings` to push; validates
  `--branch` is incompatible with `--all-projects`.
- `commands/storage.py` -- adds `--if-not-exists` to create-table.
- `server/routers/storage.py` -- `CreateTable` request model gains
  `if_not_exists: bool`; the router forwards it. Sync routes
  intentionally absent (sync is filesystem-local; documented exemption
  per CONTRIBUTING.md plugin-sync map).

Tests:
- `tests/test_storage_write.py::TestCreateTableIfNotExists` (5 cases):
  skip on existing when flag set, reraise when unset, reraise when
  target table missing despite flag, reraise on non-duplicate errors
  even with flag, success path unchanged with flag.
- `tests/test_sync_service.py::TestBranchOverrideAndNameDriftFlag`
  (4 cases): resolver priority (override wins), push branch_override
  reaches client, diff branch_override reaches client,
  no_name_drift_warnings suppresses the field from the envelope
  (with a control-arm check that proves the warning surfaces by
  default).
- One existing test in `test_storage_write.py` updated to include the
  new `if_not_exists=False` kwarg in its `assert_called_once_with`.

Live validation against project 1143 (99_Playground_Max), branch 388072:
- `sync diff --branch 388072` reaches the dev branch and reports
  `remote_only: 31`; without `--branch`, same call reports no remote
  diff.
- `storage create-table --if-not-exists` end-to-end: first call returns
  `action: "created"`; second call (same name) returns
  `action: "skipped", skip_reason: "table already exists"`; third call
  WITHOUT the flag returns the original `STORAGE_JOB_FAILED` error
  envelope.

Full test suite (3610 passed, 110 skipped) green; `ty check` clean;
`ruff check` + `ruff format --check` clean.
…storage ergonomics

Bumps version to 0.47.0 and walks the full silent-drift sync map
mandated by CONTRIBUTING.md §17 + §322-425 for the three feature
commits already on this branch:

  aaf83bc  fix(sync push): writeback placeholder manifest entries in
           place + propagate KBC.* metadata on create
  c6ce7ad  feat(semantic-layer): add search-context + get-context for
           project-wide reads
  40df5fa  feat(sync,storage): add --branch override, --if-not-exists,
           --no-name-drift-warnings

Version + auto-regenerated artefacts (CI-checked):
- pyproject.toml: 0.46.1 -> 0.47.0
- .claude-plugin/marketplace.json + plugins/kbagent/.claude-plugin/plugin.json
  re-synced via `make version-sync`
- plugins/kbagent/skills/kbagent/SKILL.md decision table regenerated
  via `make skill-gen` (now lists search-context and get-context)
- src/keboola_agent_cli/changelog.py: new 0.47.0 entry covering all
  three fixes + the no-sync-router exemption note
- uv.lock: keboola-agent-cli pin advanced to 0.47.0

Hand-maintained surfaces (silent-drift risks; not CI-checked):
- src/keboola_agent_cli/commands/context.py AGENT_CONTEXT --
  sync push/pull/diff signatures updated for --branch / --no-name-
  drift-warnings; storage create-table gains --if-not-exists;
  semantic-layer search-context / get-context added.
- CLAUDE.md `## All CLI Commands` -- storage create-table gains
  --if-not-exists; semantic-layer search-context + get-context added.
  (Sync commands are not currently listed in this file -- pre-existing
  gap, out of scope for this PR.)
- plugins/kbagent/agents/keboola-expert.md Tool Selection Matrix --
  the existing semantic-layer "list models / entities" row points at
  search-context / get-context for project-wide glob/id lookup. The
  60 KB budget for the keboola-expert prompt is tight (closing at
  59944 bytes); the addition was kept terse rather than expanding the
  matrix with a fresh row.
- plugins/kbagent/skills/kbagent/references/commands-reference.md --
  storage create-table gains --if-not-exists note; sync push/pull/diff
  gain --branch and --no-name-drift-warnings notes; new bullets for
  semantic-layer search-context and get-context.
- plugins/kbagent/skills/kbagent/references/gotchas.md -- four new
  `(since v0.47.0)` sections: fresh-CREATE writeback contract change,
  --branch override semantics, storage --if-not-exists envelope,
  --no-name-drift-warnings opt-out, and the search-context / get-
  context MCP-parity note.
- plugins/kbagent/skills/kbagent/references/sync-workflow.md -- new
  "Per-invocation dev-branch override" and "Fresh-CREATE writeback"
  sections with worked examples.

`make check` passes clean (lint + format + skill + version + changelog
+ error-codes + 3610 tests).

Pre-existing PR-body-relevant exemptions documented in changelog:
- `kbagent sync push/pull/diff` (and their new --branch flag) remain
  filesystem-local and intentionally have no REST router in
  src/keboola_agent_cli/server/routers/. Permitted by the CONTRIBUTING
  Plugin Synchronization map ("terminal-only / filesystem-bound
  commands"). All other new surfaces (storage create-table, semantic-
  layer search-context / get-context) are exposed 1:1 over HTTP.
…ror accumulation, skip render, E2E coverage)

Independent reviewer findings from iteration 2 (no BLOCKING; 4
NON-BLOCKING + 3 NIT in code; 2 NIT in security). All material items
addressed:

1. `_writeback_create_config_in_manifest` now matches placeholders on
   `(branch_id, component_id, path)` instead of `(component_id, path)`
   alone. Without this, a multi-branch manifest with the same logical
   config path under two branches could update the wrong branch's
   entry. The placeholder branch_id is also no longer overwritten by
   the helper -- the match already proves it's correct. New regression
   test: `test_writeback_config_does_not_match_across_branches`.

2. `_propagate_kbc_metadata` now returns the API error message on a
   non-fatal write failure (the config IS already created and the
   manifest writeback is complete; aborting the rest of the push
   mid-loop was the worse failure mode). The push loop accumulates the
   message into the existing `errors[]` list under a new
   `change_type: "metadata_propagation"` entry so callers can see what
   went wrong without losing the rest of the push. Added a docstring
   "not a secret store" note about KBC.* keys. New unit test:
   `test_propagate_kbc_metadata_returns_error_message_on_api_failure`.

3. `kbagent storage create-table --if-not-exists` human-mode renderer
   now prints "Skipped (already exists): <table_id>" + the reason when
   `result["action"] == "skipped"`, instead of the misleading
   "Created table: ..." line. JSON envelope unchanged. New CLI test:
   `test_human_renders_skip_when_action_is_skipped`.

4. E2E coverage in `tests/test_e2e.py::TestE2E_0_47_0_NewSurfaces`:
   - `storage create-table --if-not-exists` round-trip: created ->
     skipped -> raises without flag (binds the action envelope shape
     and the STORAGE_JOB_FAILED reraise path).
   - `semantic-layer search-context` envelope shape + type filter
     narrowing; `get-context` NOT_FOUND on all-zero UUID; roundtrip
     search -> get when the project has at least one searchable
     entity.
   - `sync diff --branch <id>`: creates a throwaway dev branch on the
     fly, asserts diff reaches that branch (status=ok, changes != None,
     remote_only >= 0), cleans up the dev branch in teardown.

Items kept but not changed:
- Pyright lambda-parameter noise in tests (`url`/`token` "not accessed"):
  pre-existing across the test suite, idiomatic for the client_factory
  signature; ty is clean.
- FastAPI router `type` parameter name in `semantic_layer.search-context`:
  cosmetic shadow of the builtin; ignoring.

`make check` clean: 3613 passed, 7 skipped, 106 deselected.
`make test-e2e-local CONFIG_DIR=/tmp/kbc-config-e2e ALIAS=e2e-1143
PYTEST_ARGS=...TestE2E_0_47_0_NewSurfaces` -- all 3 new e2e tests pass
against project 1143 (529s; 67 passed, 6 skipped, 0 failed total when
running the broader e2e suite).
…tring off-by-one)

Iteration 3 (independent convergence reviewer) returned "CONVERGED -- zero
material findings." Two documentation NITs noted and fixed here:

- changelog.py:12 -- the 0.47.0 entry's prose still said
  `_writeback_create_config_in_manifest` matches placeholders by
  `(component_id, path)`. Iteration 2 narrowed the key to
  `(branch_id, component_id, path)` for multi-branch safety; the
  changelog now reflects the final key and notes the why.
- tests/test_e2e.py docstring on TestE2E_0_47_0_NewSurfaces -- said
  "All four touch a real Keboola project" but the class has three test
  methods. Fixed to "All three".

Iteration 3 also flagged one pre-existing scope item that iteration 2
did NOT introduce:

- `storage create-table --if-not-exists` skipped-envelope reports the
  USER-REQUESTED `primary_key` / `columns` (from the create call's
  args), not the EXISTING table's actual schema. A caller that relies
  on the envelope to discover the real schema would get the wrong
  shape. Out of scope for this PR; will be filed as a separate
  follow-up issue against keboola/cli before merge per the deferred-
  scope-orphan rule.

`make check` clean: 3613 passed, 7 skipped, 106 deselected.

Branch is convergence-clean. Next: pause for user authorization before
opening the PR.
…nst real API

Closes the most important e2e coverage gap that the user flagged:
the Area B headline fix (writeback in place + KBC.configuration.*
metadata propagation on CREATE) was only live-validated manually in
the earlier session; nothing in the test suite would catch a
regression.

New test `TestE2E_0_47_0_NewSurfaces::test_sync_push_fresh_create_writeback_and_kbc_metadata`:
- Creates a throwaway dev branch on the configured project.
- `sync init` then `sync pull --branch <dev>` so the dev branch lands
  in the manifest.
- Hand-authors a placeholder ManifestConfiguration with
  `KBC.configuration.folderName` declared (FIIA / scaffold pattern),
  writes a matching `_config.yml` locally.
- `sync push --branch <dev>` -- asserts `created=1, errors=[]`.
- Manifest invariants: length unchanged (writeback in place, NO
  duplicate), placeholder id replaced with the API-assigned ULID,
  `KBC.configuration.folderName` preserved on the entry under the
  right `branch_id`.
- `config metadata-list` against the new id verifies the folderName
  landed on the remote via the metadata API.
- `sync push` second invocation -- asserts `created=0` (idempotent).
- Teardown deletes the dev branch (and with it every config created
  inside it) so re-runs don't accumulate residue.

Also: `yaml` import added at the top of `tests/test_e2e.py` -- the
file uses `yaml.dump` in the placeholder fixture builder.

Live validated:
- New test passes against project 1143 (e2e-1143 / 99_Playground_Max)
  in 9.44s (direct pytest invocation).
- Full e2e suite still 67 passed, 6 skipped; the one failing test
  (`TestFullE2E::test_full_cli_e2e::_test_file_operations`) is an
  unrelated pre-existing flake against the Storage Files index lag
  and is not introduced by this change.
Copy link
Copy Markdown
Contributor Author

@ottomansky ottomansky 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 #348 — feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics

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, not duplicated here.

Summary

This PR bundles three coherent additions for v0.47.0: (A) fresh-CREATE manifest writeback correctness (placeholder updated in place, KBC.configuration.* metadata propagated), (B) semantic-layer search-context and get-context commands mirroring the MCP search_semantic_context / get_semantic_context tools, and (C) sync/storage ergonomics (sync push|pull|diff --branch, storage create-table --if-not-exists, sync push --no-name-drift-warnings). The plugin sync-map was walked thoroughly — the vast majority of surfaces are correctly updated. Two gaps remain: the keboola-expert.md §1 Rule 6 VERSION GATE block was not extended for the 0.47.0 additions, and the follow-up issue promised in the PR body before merge (stale columns/primary_key in the --if-not-exists skipped envelope) has not been filed. Verdict is REQUEST CHANGES solely because of the VERSION GATE gap; the untracked follow-up issue is NON-BLOCKING per the deferred-scope-orphan-prevention rule.

Verdict

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

Blocking findings

[B-1] plugins/kbagent/agents/keboola-expert.md:65-118 — §1 Rule 6 VERSION GATE not updated for 0.47.0 features

The VERSION GATE block ends at kbagent agent <verb> (CLI parity /agents REST) = 0.44.0+. None of the four new 0.47.0 surfaces are listed there: semantic-layer search-context, semantic-layer get-context, storage create-table --if-not-exists, and sync push|pull|diff --branch. The §2 Tool Selection Matrix was updated (one row amended to mention search-context / get-context (0.47.0+)), but the VERSION GATE is the machine-readable gate that the keboola-expert agent checks at session start before attempting a task — it is the layer that causes the agent to refuse the task and emit a repair path when the installed kbagent is too old. Without entries here, an agent running on kbagent 0.46.x will attempt semantic-layer search-context, receive No such command, and either crash or silently fall back to MCP, contradicting the stated "MCP-parity so downstream callers can drop the MCP dependency" design goal.

Fix: add a 0.47.0 block to the VERSION GATE list (after the 0.44.0+ entry, before the closing sentence) along the lines of:

   `semantic-layer search-context` / `get-context` (project-wide glob/id lookup) = 0.47.0+,
   `storage create-table --if-not-exists` (idempotency for parallel workers) = 0.47.0+,
   `sync push|pull|diff --branch <id>` (per-invocation dev-branch override) = 0.47.0+,

Non-blocking findings

[NB-1] PR body — follow-up issue for stale columns/primary_key in --if-not-exists skipped envelope not filed

The PR body states "To be filed against keboola/cli before merge" for the known design limitation where the action: "skipped" envelope returns the requested columns and primary_key rather than the existing table's actual schema. As of this review the issue has not been filed (confirmed by searching keboola/cli open issues). Per the repo's deferred-scope-orphan-prevention rule, the follow-up issue must be filed and linked from the PR body BEFORE merge, not after. This is distinct from the functional correctness of the code — the behavior is intentional — but the tracking discipline is required.

Fix: file the GitHub issue, paste the number into the "Follow-up issues to file before merge" section of the PR body (replacing "TBD"), and link it from gotchas.md's --if-not-exists entry so that future operators probing the skipped envelope for schema discovery see the caveat inline.

[NB-2] plugins/kbagent/skills/kbagent/references/gotchas.md:59-70--if-not-exists skipped envelope caveat undocumented

The gotchas.md entry for storage create-table --if-not-exists (since v0.47.0) correctly documents the happy-path behavior but omits the known limitation: when action: "skipped", the columns and primary_key fields in the returned envelope reflect the user-requested values, not the existing table's actual schema. An AI agent that reads columns from the skipped envelope to make downstream decisions (e.g., "is the existing table's schema compatible with my next transformation?") will silently operate on stale data. The PR description explicitly calls this out as pre-existing design scope, which means it's a real gotcha for agent consumers.

Fix: add a sentence to the existing gotcha entry: "Note: when action: "skipped", the columns and primary_key fields reflect the requested (input) values, not the existing table's actual schema. Call storage table-detail to read the real schema." This is independent of whether a follow-up issue is filed.

Nits

  • [NIT-1] src/keboola_agent_cli/services/semantic_layer_service.py:304import fnmatch is buried inside the _matches_any_pattern static method body. The rest of the codebase uses top-level imports (see permissions.py:8 which imports fnmatch at module level). Moving it to the top-level import block alongside json, logging, re is a one-line change that removes the cognitive speed-bump of a local import.

Verification log

  • gh auth status → authenticated as ottomansky, scopes include repo
  • git rev-parse --abbrev-ref HEADfeat/fresh-create-completeness ✓ (matches <branch>)
  • gh pr view 348 --json stateOPEN
  • gh pr diff 348 > /tmp/kbagent-pr-348.diff → 3322 lines, 27 changed files, +2675/-57 ✓
  • Layer check (typer/formatter in services): grep -E '^\+(from typer|import typer|...)' diff | grep services/ → empty ✓
  • Layer check (httpx in commands): grep -E '^\+(from httpx|httpx\.)' diff | grep commands/ → empty ✓
  • make check → 3613 passed, 7 skipped, 107 deselected, 0 failed, exit 0 ✓
  • make typecheck → 1 warning unresolved-import: hatchling.builders.hooks.plugin.interface in scripts/hatch_build.py:61 — pre-existing, NOT from this PR; no new type errors ✓
  • OPERATION_REGISTRY check: grep -n 'search.context\|get.context' permissions.py"semantic-layer.search-context": "read" at line 177, "semantic-layer.get-context": "read" at line 178 ✓
  • AGENT_CONTEXT check: grep -n 'search.context\|get.context' commands/context.py → both listed with (since 0.47.0) tags ✓
  • CLAUDE.md ## All CLI Commands check: grep -n 'search.context\|get.context' CLAUDE.md → both present at lines 435-436 ✓
  • keboola-expert.md §2 Tool Selection Matrix check: amended row at line 186 adds search-context / get-context (0.47.0+)
  • keboola-expert.md §1 Rule 6 VERSION GATE check: last version entry is 0.44.0+; no 0.47.0 additions found → BLOCKING
  • commands-reference.md check: grep -n 'search.context\|get.context' commands-reference.md → both at lines 209-210 with (since 0.47.0)
  • gotchas.md check: grep -n 'since v0.47' gotchas.md → 5 entries, all with version tags ✓; stale-columns caveat absent from if-not-exists entry → NON-BLOCKING ✗
  • sync-workflow.md check: updated with --branch and fresh-CREATE sections ✓
  • hints/definitions/semantic_layer.py check: search-context at line 165, get-context at line 204 ✓
  • server/routers/storage.py check: if_not_exists: bool = False added to CreateTable model ✓
  • server/routers/semantic_layer.py check: diff shows search-context and get-context routes added ✓
  • Follow-up issue (stale skipped envelope): gh issue list --search "if-not-exists" → no matching issue found; PR body still shows "TBD" → NON-BLOCKING (deferred-scope-orphan-prevention rule) ✗
  • Behavior verification: author-provided live validation receipts accepted (reviewer does not have credentials to project 1143 / 99_Playground_Max); receipts cover all 7 new surfaces with positive and idempotent passes ✓
  • Magic numbers / bare except / raw error strings / print() in production: none found ✓
  • Token security: no unmasked token references in new code ✓

Open questions for the author

(none)

…-up issue, gotchas caveat, fnmatch import)

The /kbagent:review subagent caught one BLOCKING and three lower-
severity findings that the iteration-2 + iteration-3 independent
reviewers had missed. All four addressed here:

[B-1] keboola-expert.md §1 Rule 6 VERSION GATE not updated for 0.47.0.
  An agent on 0.46.x would attempt `semantic-layer search-context` /
  `get-context` / `storage create-table --if-not-exists` /
  `sync push|pull|diff --branch` and get "No such command", silently
  losing the stated MCP-parity benefit. Added a single 0.47.0+ row
  covering all four new surfaces + the fresh-CREATE writeback +
  KBC.* propagation behavior change. Stayed under the 60000-byte
  prompt budget by also tightening the verbose 0.41.0 `semantic-layer`
  build-heuristic note from a five-line wall to a one-line inline.

[NB-1] PR body had `TBD` for the deferred-scope follow-up issue.
  Filed keboola#349 with a complete repro + suggested fix shape
  and updated the PR body to link it. Tracking the design-surface
  scope outside this PR per the deferred-scope-orphan-prevention rule.

[NB-2] gotchas.md `--if-not-exists` entry documented the happy path
  but did not warn that the skipped envelope's `columns` /
  `primary_key` mirror the user's REQUEST, not the EXISTING table's
  actual schema. Added an explicit caveat referencing keboola#349
  and pointing callers at `storage table-detail` if they need the
  real shape.

[NIT] `import fnmatch` inline inside `_matches_any_pattern` static
  method body. Hoisted to the top-level imports in
  `semantic_layer_service.py` for consistency with `permissions.py`
  and the rest of the file.

`make check` clean (3613 passed, 7 skipped). `tests/test_agent_prompt.py`
budget check green (60000-byte ceiling respected). `ty check` clean.
Copy link
Copy Markdown
Contributor Author

@ottomansky ottomansky 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 #348 — feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics

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, not duplicated here.

Summary

This PR delivers three coherent 0.47.0 additions: (A) sync push fresh-CREATE writeback that updates placeholder manifest entries in-place instead of appending duplicates, (B) semantic-layer search-context + get-context for project-wide context reads replacing the MCP dependency, and (C) ergonomic flags (sync --branch, storage create-table --if-not-exists, sync push --no-name-drift-warnings). The four findings from the prior /kbagent:review iteration-3 pass (B-1 through NIT) are all confirmed resolved in commit 51c0684. The independent fresh pass surfaces one NON-BLOCKING finding (semantic_layer_service.py crossed the hard file-size ceiling during this PR) and one NIT (storage create-table hint definition missing if_not_exists in ServiceCall args). The overall quality is high: 3613 tests pass, 4 new E2E tests, all plugin sync-map surfaces updated, and make typecheck exits 0.

Verdict: APPROVE

Verdict

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

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/services/semantic_layer_service.py — hard file-size ceiling crossed during this PR

Before this PR: semantic_layer_service.py was 1480 LOC (just under the hard ceiling of 1500 LOC for services/). After this PR it is 1640 LOC — 9% over the hard ceiling. Per CONTRIBUTING.md "File-size budgets" section: "When a file crosses the hard ceiling, splitting is required before merging more functionality into it." The new search_context / get_context methods and the _matches_any_pattern / _strip_semantic_prefix statics (plus class-level constants _ALL_TYPES_FOR_LOOKUP) are the cleanest split candidates — they share no state with the model-scoped CRUD methods and form a natural "project-wide lookup" sub-concern. A sibling semantic_layer_context_service.py (or _semantic_layer_lookup.py helper) would bring both files under the soft ceiling. Not blocking because the feature itself is correct and the existing test/lint suite passes; but the next PR that adds to this file will hit the rule again.

Nits

  • [NIT-1] src/keboola_agent_cli/hints/definitions/storage.py:222-234 — the CommandHint for storage.create-table lists ServiceCall args but does not include if_not_exists and has no note about the flag. An AI agent invoking kbagent --hint service storage create-table ... --if-not-exists generates code that silently drops the idempotency flag. Recommend adding "if_not_exists": "{if_not_exists}" to the ServiceCall.args dict and a one-liner note: "Pass if_not_exists=True for parallel-worker idempotency (since 0.47.0); response gains action: 'created'|'skipped'." This is cosmetic/advisory; does not affect runtime behavior.

Verification log

  • gh pr view 348 --json title,body,files,additions,deletions,baseRefName,headRefName,state → 27 files, +2686 additions, OPEN, feat/fresh-create-completeness -> main
  • git rev-parse --abbrev-ref HEADfeat/fresh-create-completeness (working tree matches reviewed branch) ✓
  • CONTRIBUTING.md Plugin synchronization map read, all "NO"-column rows walked ✓
  • CLAUDE.md convention #17 read ✓
  • plugins/kbagent/agents/keboola-expert.md §1, §2, §3 read ✓
  • make check → 3613 passed, 7 skipped, 107 deselected, 12 warnings, exit 0 ✓
  • make typecheckwarning[unresolved-import] on pre-existing scripts/hatch_build.py:61 (downgraded rule per CONTRIBUTING.md, non-blocking), exit 0 ✓

Iteration-4 fix verification (commit 51c0684):

  • [B-1] VERSION GATE: git show 51c0684 -- plugins/kbagent/agents/keboola-expert.md → confirms addition of semantic-layer search-context|get-context, storage create-table --if-not-exists, sync push|pull|diff --branch, sync push --no-name-drift-warnings, fresh-CREATE writeback + KBC.* = 0.47.0+` row at line 113 ✓
  • [NB-1] Deferred issue: gh issue view 349 -R keboola/cli → issue OPEN, title "storage create-table --if-not-exists: skipped envelope returns requested schema, not actual existing table schema", gh pr view 348 --json body | grep 349 → count 1 ✓. PR body links the issue.
  • [NB-2] gotchas.md caveat: git show 51c0684 -- plugins/kbagent/skills/kbagent/references/gotchas.md → 9-line **Caveat — skipped envelope returns REQUESTED schema, not ACTUAL schema** block added immediately after the --if-not-exists section, references #349, points callers to storage table-detail
  • [NIT] fnmatch import: grep -n 'import fnmatch' src/keboola_agent_cli/services/semantic_layer_service.py → line 16, top-level import confirmed ✓

Fresh independent pass — Plugin synchronization map:

  • commands/context.py AGENT_CONTEXT: grep 'search.context\|get.context\|if.not.exists\|no.name.drift\|--branch.*sync' commands/context.py → all four surfaces documented at correct version ✓
  • CLAUDE.md ## All CLI Commands: grep 'search.context\|get.context\|if.not.exists' CLAUDE.md → both new semantic-layer commands and --if-not-exists flag present ✓
  • permissions.py OPERATION_REGISTRY: grep 'search.context\|get.context' permissions.py → lines 177-178, both registered as "read"
  • hints/definitions/semantic_layer.py: search-context at line 165, get-context at line 204, both with ClientCall + ServiceCall ✓
  • hints/definitions/storage.py create-table hint: ServiceCall args missing if_not_exists → NIT-1 above
  • server/routers/semantic_layer.py: grep 'search.context\|get.context' → GET routes at lines 259, 277 ✓
  • server/routers/storage.py: grep 'if_not_exists' → lines 34, 241, parameter propagated ✓
  • gotchas.md: 4 new (since v0.47.0) sections confirmed present and properly version-tagged ✓
  • commands-reference.md: sync push/pull/diff --branch documented at lines 189-191; semantic-layer search-context at line 209, get-context at line 210 ✓
  • sync-workflow.md: grep 'branch.*override\|fresh.create' → sections at lines 70, 100 ✓
  • keboola-expert.md §2 Tool Selection Matrix: row for "List models / metrics / entities" updated to include search-context / get-context (0.47.0+)
  • keboola-expert.md §3 Inline Gotchas: no new 0.47.0-specific inline gotcha needed (all documented in dedicated gotchas.md and referenced from §3's semantic-layer block) ✓

3-layer compliance:

  • grep typer|click in services diff → empty ✓
  • grep httpx in commands diff → empty ✓
  • grep formatter\.|console.print in client diff → empty ✓

Test coverage:

  • test_semantic_layer_service.py: TestSearchContext (12 methods) + TestGetContext (6 methods) ✓
  • test_semantic_layer_cli.py: TestSearchContext (4 methods) + TestGetContext (3 methods) ✓
  • test_storage_write.py: --if-not-exists service-layer + CLI tests confirmed ✓
  • test_sync_service.py: TestFreshCreateWriteback present ✓
  • test_e2e.py: 4 new E2E tests — test_storage_create_table_if_not_exists_round_trip, test_semantic_layer_search_and_get_context, test_sync_diff_branch_override, test_sync_push_fresh_create_writeback_and_kbc_metadata

File-size audit:

  • semantic_layer_service.py: 1480 LOC pre-PR → 1640 LOC post-PR (hard ceiling 1500) → NB-1 above
  • sync_service.py: 2767 LOC pre-PR → 2930 LOC (already over hard ceiling before this PR; grandfathered per CONTRIBUTING.md §"File-size budgets" reference note)
  • storage_service.py: 2291 → 2327 LOC (same grandfathered situation)
  • commands/storage.py: 2611 → 2631 LOC (same grandfathered situation)

Behavior verification:

  • storage create-table --if-not-exists: confirmed logic at storage_service.py:799-822 catches STORAGE_JOB_FAILED + "already has the same display name", probes get_table_detail(target_id), returns action: "skipped" or re-raises. No raw error-code string (uses ErrorCode.STORAGE_JOB_FAILED). make check-error-codes would enforce this.
  • search_context / get_context: confirmed finally: client.close() on both paths; empty-pattern and zero-limit rejected at service boundary; non-404 errors propagate immediately in get_context
  • _writeback_create_config_in_manifest: matches on (branch_id, component_id, path) tuple — branch included to guard against multi-branch manifests with identical paths ✓
  • _propagate_kbc_metadata: filters by KBC. prefix, accumulates errors (non-fatal) ✓
  • Live validation: author's changelog entry documents live validation against project 1143 / dev branch 388072 for all three areas ✓

Open questions for the author

(none)

Copy link
Copy Markdown
Contributor Author

@ottomansky ottomansky 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 #348 — feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics

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, not duplicated here.

Summary

This PR delivers three coherent 0.47.0 additions: (A) sync push fresh-CREATE writeback that updates placeholder manifest entries in-place instead of appending duplicates, (B) semantic-layer search-context + get-context for project-wide context reads replacing the MCP dependency, and (C) ergonomic flags (sync --branch, storage create-table --if-not-exists, sync push --no-name-drift-warnings). The four findings from the prior /kbagent:review iteration-3 pass (B-1 through NIT) are all confirmed resolved in commit 51c0684. The independent fresh pass surfaces one NON-BLOCKING finding (semantic_layer_service.py crossed the hard file-size ceiling during this PR) and one NIT (storage create-table hint definition missing if_not_exists in ServiceCall args). The overall quality is high: 3613 tests pass, 4 new E2E tests, all plugin sync-map surfaces updated, and make typecheck exits 0.

Verdict: APPROVE

Verdict

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

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/services/semantic_layer_service.py — hard file-size ceiling crossed during this PR

Before this PR: semantic_layer_service.py was 1480 LOC (just under the hard ceiling of 1500 LOC for services/). After this PR it is 1640 LOC — 9% over the hard ceiling. Per CONTRIBUTING.md "File-size budgets" section: "When a file crosses the hard ceiling, splitting is required before merging more functionality into it." The new search_context / get_context methods and the _matches_any_pattern / _strip_semantic_prefix statics (plus class-level constants _ALL_TYPES_FOR_LOOKUP) are the cleanest split candidates — they share no state with the model-scoped CRUD methods and form a natural "project-wide lookup" sub-concern. A sibling semantic_layer_context_service.py (or _semantic_layer_lookup.py helper) would bring both files under the soft ceiling. Not blocking because the feature itself is correct and the existing test/lint suite passes; but the next PR that adds to this file will hit the rule again.

Nits

  • [NIT-1] src/keboola_agent_cli/hints/definitions/storage.py:222-234 — the CommandHint for storage.create-table lists ServiceCall args but does not include if_not_exists and has no note about the flag. An AI agent invoking kbagent --hint service storage create-table ... --if-not-exists generates code that silently drops the idempotency flag. Recommend adding "if_not_exists": "{if_not_exists}" to the ServiceCall.args dict and a one-liner note: "Pass if_not_exists=True for parallel-worker idempotency (since 0.47.0); response gains action: 'created'|'skipped'." This is cosmetic/advisory; does not affect runtime behavior.

Verification log

  • gh pr view 348 --json title,body,files,additions,deletions,baseRefName,headRefName,state → 27 files, +2686 additions, OPEN, feat/fresh-create-completeness -> main
  • git rev-parse --abbrev-ref HEADfeat/fresh-create-completeness (working tree matches reviewed branch) ✓
  • CONTRIBUTING.md Plugin synchronization map read, all "NO"-column rows walked ✓
  • CLAUDE.md convention #17 read ✓
  • plugins/kbagent/agents/keboola-expert.md §1, §2, §3 read ✓
  • make check → 3613 passed, 7 skipped, 107 deselected, 12 warnings, exit 0 ✓
  • make typecheckwarning[unresolved-import] on pre-existing scripts/hatch_build.py:61 (downgraded rule per CONTRIBUTING.md, non-blocking), exit 0 ✓

Iteration-4 fix verification (commit 51c0684):

  • [B-1] VERSION GATE: git show 51c0684 -- plugins/kbagent/agents/keboola-expert.md → confirms addition of semantic-layer search-context|get-context, storage create-table --if-not-exists, sync push|pull|diff --branch, sync push --no-name-drift-warnings, fresh-CREATE writeback + KBC.* = 0.47.0+` row at line 113 ✓
  • [NB-1] Deferred issue: gh issue view 349 -R keboola/cli → issue OPEN, title "storage create-table --if-not-exists: skipped envelope returns requested schema, not actual existing table schema", gh pr view 348 --json body | grep 349 → count 1 ✓. PR body links the issue.
  • [NB-2] gotchas.md caveat: git show 51c0684 -- plugins/kbagent/skills/kbagent/references/gotchas.md → 9-line **Caveat — skipped envelope returns REQUESTED schema, not ACTUAL schema** block added immediately after the --if-not-exists section, references #349, points callers to storage table-detail
  • [NIT] fnmatch import: grep -n 'import fnmatch' src/keboola_agent_cli/services/semantic_layer_service.py → line 16, top-level import confirmed ✓

Fresh independent pass — Plugin synchronization map:

  • commands/context.py AGENT_CONTEXT: grep 'search.context\|get.context\|if.not.exists\|no.name.drift\|--branch.*sync' commands/context.py → all four surfaces documented at correct version ✓
  • CLAUDE.md ## All CLI Commands: grep 'search.context\|get.context\|if.not.exists' CLAUDE.md → both new semantic-layer commands and --if-not-exists flag present ✓
  • permissions.py OPERATION_REGISTRY: grep 'search.context\|get.context' permissions.py → lines 177-178, both registered as "read"
  • hints/definitions/semantic_layer.py: search-context at line 165, get-context at line 204, both with ClientCall + ServiceCall ✓
  • hints/definitions/storage.py create-table hint: ServiceCall args missing if_not_exists → NIT-1 above
  • server/routers/semantic_layer.py: grep 'search.context\|get.context' → GET routes at lines 259, 277 ✓
  • server/routers/storage.py: grep 'if_not_exists' → lines 34, 241, parameter propagated ✓
  • gotchas.md: 4 new (since v0.47.0) sections confirmed present and properly version-tagged ✓
  • commands-reference.md: sync push/pull/diff --branch documented at lines 189-191; semantic-layer search-context at line 209, get-context at line 210 ✓
  • sync-workflow.md: grep 'branch.*override\|fresh.create' → sections at lines 70, 100 ✓
  • keboola-expert.md §2 Tool Selection Matrix: row for "List models / metrics / entities" updated to include search-context / get-context (0.47.0+)
  • keboola-expert.md §3 Inline Gotchas: no new 0.47.0-specific inline gotcha needed (all documented in dedicated gotchas.md and referenced from §3's semantic-layer block) ✓

3-layer compliance:

  • grep typer|click in services diff → empty ✓
  • grep httpx in commands diff → empty ✓
  • grep formatter\.|console.print in client diff → empty ✓

Test coverage:

  • test_semantic_layer_service.py: TestSearchContext (12 methods) + TestGetContext (6 methods) ✓
  • test_semantic_layer_cli.py: TestSearchContext (4 methods) + TestGetContext (3 methods) ✓
  • test_storage_write.py: --if-not-exists service-layer + CLI tests confirmed ✓
  • test_sync_service.py: TestFreshCreateWriteback present ✓
  • test_e2e.py: 4 new E2E tests — test_storage_create_table_if_not_exists_round_trip, test_semantic_layer_search_and_get_context, test_sync_diff_branch_override, test_sync_push_fresh_create_writeback_and_kbc_metadata

File-size audit:

  • semantic_layer_service.py: 1480 LOC pre-PR → 1640 LOC post-PR (hard ceiling 1500) → NB-1 above
  • sync_service.py: 2767 LOC pre-PR → 2930 LOC (already over hard ceiling before this PR; grandfathered per CONTRIBUTING.md §"File-size budgets" reference note)
  • storage_service.py: 2291 → 2327 LOC (same grandfathered situation)
  • commands/storage.py: 2611 → 2631 LOC (same grandfathered situation)

Behavior verification:

  • storage create-table --if-not-exists: confirmed logic at storage_service.py:799-822 catches STORAGE_JOB_FAILED + "already has the same display name", probes get_table_detail(target_id), returns action: "skipped" or re-raises. No raw error-code string (uses ErrorCode.STORAGE_JOB_FAILED). make check-error-codes would enforce this.
  • search_context / get_context: confirmed finally: client.close() on both paths; empty-pattern and zero-limit rejected at service boundary; non-404 errors propagate immediately in get_context
  • _writeback_create_config_in_manifest: matches on (branch_id, component_id, path) tuple — branch included to guard against multi-branch manifests with identical paths ✓
  • _propagate_kbc_metadata: filters by KBC. prefix, accumulates errors (non-fatal) ✓
  • Live validation: author's changelog entry documents live validation against project 1143 / dev branch 388072 for all three areas ✓

Open questions for the author

(none)

…gistry)

The second /kbagent:review pass returned APPROVE with two new findings
the prior reviewers hadn't surfaced. Both addressed here:

[NB-1] services/semantic_layer_service.py crossed the CONTRIBUTING.md
  hard ceiling (1500 LOC for services/*.py): 1480 -> 1640 LOC during
  this PR. Extracted search_context + get_context into a new sibling
  helper `services/_semantic_layer_lookup.py` following the existing
  `_semantic_layer_crud.py` / `_semantic_layer_internals.py` / etc.
  pattern. The helpers now own the metastore client lifecycle (open +
  finally close) via an `open_client: Callable[[], MetastoreClient]`
  factory the service injects with a 1-line lambda; the service
  methods are pure 1-line delegators.

  semantic_layer_service.py: 1640 -> 1496 LOC (under the hard ceiling).
  _semantic_layer_lookup.py: new file, 187 LOC.

  Two minor banner-comment trims (`# Helpers (used by every subcommand)`,
  `# Phase 3 — Read commands`) collapsed to single inline comments to
  bring the count just under the 1500 ceiling without changing any
  behavior or structure.

[NIT-1] hints/definitions/storage.py `create-table` `ServiceCall` args
  was missing `if_not_exists`. An AI agent following `--hint service`
  would generate non-idempotent code even when the caller wanted the
  IF-NOT-EXISTS path. Added the arg + a `notes[]` line documenting
  the 0.47.0+ flag.

Verification:
- `make check` clean: 3613 passed, 7 skipped, 107 deselected
- `ty check` clean
- The two existing e2e tests touched by this change
  (test_semantic_layer_search_and_get_context,
  test_sync_push_fresh_create_writeback_and_kbc_metadata) re-run
  against project 1143 and still pass in 13.67s
- The Pyright "Import could not be resolved" diagnostics on the new
  `_semantic_layer_lookup` import are stale-cache artifacts; ty is
  the project's authoritative typechecker and it is green.

Helper-design choice: the extraction inverts the client-lifecycle
ownership (helpers open + close vs. service open + pass-in close).
This matters because the service methods become genuinely 1-line and
the orchestrator class stays well under budget for future growth.
The cost is one extra import (`Callable` via TYPE_CHECKING) in the
helper; gain is ~50 LOC saved in the orchestrator on top of the
~140 LOC moved out.
Copy link
Copy Markdown
Contributor Author

@ottomansky ottomansky 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 #348 — feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics

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, not duplicated here.

Summary

This PR bundles v0.47.0 across three coherent areas: (B) sync-push fresh-CREATE placeholder writeback + KBC.* metadata propagation, (A) two new semantic-layer read commands mirroring MCP parity, and (C) ergonomics flags across sync and storage create-table. The implementation is clean, the plugin synchronization map is fully walked, and iteration-5 commit 0d63342 correctly resolves both prior findings: semantic_layer_service.py is now at 1496 LOC (hard ceiling: 1500) and hints/definitions/storage.py includes if_not_exists in the ServiceCall args. Verdict: APPROVE.

Verdict

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

Blocking findings

(none)

Non-blocking findings

(none)

Nits

  • [NIT-1] src/keboola_agent_cli/services/_semantic_layer_lookup.py:104,160try/finally with explicit client.close() instead of with open_client() as client:. MetastoreClient implements __enter__/__exit__, so with open_client() as client: is the CONTRIBUTING.md-preferred form ("Use a context manager every time the resource has __enter__/__exit__"). The try/finally pattern is functionally equivalent and not a resource-leak risk (client is assigned before the try block so a factory failure never leaves an open handle; close() fires in finally on both early return and exception). The pattern was extracted from semantic_layer_service.py which uses try/finally throughout — fixing it here without fixing the parent file would create asymmetry, so this is best addressed as a follow-up refactor of the whole service rather than only the new helper. No action needed before merge.

Verification log

  • gh auth status → authenticated as ottomansky, token scopes sufficient ✓
  • gh pr view 348 --json statestate: OPEN
  • git rev-parse --abbrev-ref HEADfeat/fresh-create-completeness matches PR branch ✓
  • wc -l /tmp/kbagent-pr-348.diff → 3483 lines, 29 files, +2749/-82 ✓
  • Iteration-5 fix NB-1: wc -l services/semantic_layer_service.py1496 (hard ceiling 1500) ✓
  • Iteration-5 fix NIT-1: grep "if_not_exists" hints/definitions/storage.py → line 235: "if_not_exists": "{if_not_exists}" in ServiceCall args, plus notes[5] documents 0.47.0+ behavior ✓
  • Layer compliance: grep typer|click|formatter|httpx in new service/client files → empty; no layer violations in diff ✓
  • Bare except / magic numbers / raw error codes: all grep patterns → empty ✓
  • OPERATION_REGISTRY (permissions.py): "semantic-layer.search-context": "read" at line 177, "semantic-layer.get-context": "read" at line 178 ✓
  • AGENT_CONTEXT (commands/context.py): search-context documented at line 827, get-context at line 835, --if-not-exists at line 345 with (since 0.47.0) note, --branch at line 778, --no-name-drift-warnings at line 786 ✓
  • CLAUDE.md ## All CLI Commands: search-context at line 435, get-context at line 436, --if-not-exists on create-table line 349 ✓
  • keboola-expert.md §1 Rule 6 VERSION GATE: single consolidated line at line 113 lists all 0.47.0 surfaces ✓
  • keboola-expert.md §2 Tool Selection Matrix: search-context / get-context surfaced in existing "List models / metrics / entities" row at line 182 (read commands; dedicated matrix row NON-BLOCKING per CONTRIBUTING.md) ✓
  • commands-reference.md: search-context at line 209, get-context at line 210, --if-not-exists on create-table at line 87 ✓
  • gotchas.md version tags: 5 new entries, all tagged (since v0.47.0) — lines 14, 49, 59, 80, 89 ✓
  • sync-workflow.md: --branch override section at line 70, fresh-CREATE writeback section at line 100 ✓
  • Hints definitions: semantic-layer.search-context at line 169, semantic-layer.get-context at line 208, if_not_exists in storage.create-table ServiceCall at line 235 ✓
  • Server routers: search_context route at semantic_layer.py:259, get_context route at line 277, if_not_exists in storage.py router body at line 34 and forwarded at line 241 ✓
  • _semantic_layer_lookup.py resource management: client = open_client() outside try, finally: client.close() at lines 127 and 179. MetastoreClient has __enter__/__exit__. Both helpers include a dedicated test verifying close() fires on API error (test_client_closed_even_on_api_error in TestSearchContext at line 2578 and TestGetContext at line 2658) ✓
  • _semantic_layer_lookup.py circular imports: only imports from ..errors at runtime; MetastoreClient and SemanticType behind TYPE_CHECKING guard — no circular import ✓
  • Backward compatibility: "action": "created" is a new additive field in the storage create-table success envelope (previously absent). No existing consumer expects its absence; HTTP router passthrough is transparent. "action": "skipped" only fires with --if-not-exists=True which defaults to False
  • Sync push result shape: branch_id is used internally but NOT added unconditionally to the return dict — result_data at line 1352 is unchanged except for the conditional name_drift_warnings key ✓
  • Test count: 48 new def test_ methods across test_semantic_layer_service.py, test_semantic_layer_cli.py, test_storage_write.py, test_sync_service.py, plus 4 E2E tests in test_e2e.py
  • mock_client.close.assert_called_once(): present in test_storage_write.py (lines 78, 112, 220, 305), in test_semantic_layer_service.py (lines 259, 2589, 2667) ✓
  • make check: 3613 passed, 7 skipped, 107 deselected, 0 failed — exit 0 ✓
  • Behavior verification: PR body contains full live-validation receipts for all seven surfaces against project 1143 / europe-west3.gcp, including the fresh-CREATE + KBC.* E2E round-trip, semantic-layer search-context with pattern filtering, semantic-layer get-context UUID miss path, sync diff --branch, storage create-table --if-not-exists ×3, and re-push idempotency. The focus-area iteration-5 items are independently confirmed as described above.

Open questions for the author

(none)

…client() as client:`

Addresses the lone NIT from the /kbagent:review iteration-5 pass.
CONTRIBUTING.md prefers the `with` form for resources that implement
`__enter__`/`__exit__`; `MetastoreClient` has had both since v0.41.0
but every method in `services/semantic_layer_service.py` was using
the older `try/finally + client.close()` idiom. The new
`_semantic_layer_lookup.py` (added in iteration 5) inherited that
pattern. The reviewer flagged the new helper but the right fix is to
sweep the whole service for consistency, not patch just the new file.

20 single-client sites converted via a small one-shot Python rewrite
(`/tmp/refactor_with.py`) that matched the canonical shape:

    client = self._new_metastore_client(project)
    try:
        ...
    finally:
        client.close()

    -->

    with self._new_metastore_client(project) as client:
        ...

Body indentation is unchanged (the try-block and the with-block use
the same +4 indent).

1 cross-project (promote) site converted by hand to the new
parenthesized multi-context-manager form:

    with (
        self._new_metastore_client(projects[from_project]) as src_client,
        self._new_metastore_client(projects[to_project]) as tgt_client,
    ):
        ...

`_semantic_layer_lookup.py::run_search_context` and `run_get_context`
also switched to `with open_client() as client:`. The factory pattern
the service injects still works: `lambda: self._new_metastore_client(
self._resolve_one_project(alias))` returns a MetastoreClient
configured per the resolved project.

Test fixture updates: `_make_service` in `test_semantic_layer_service.py`
now sets `mock.__enter__ = MagicMock(return_value=mock)` +
`mock.__exit__ = MagicMock(return_value=False)` on the injected
MagicMock so a `with` block over the mock yields the same body the
test configures side-effects on. The `TestPromoteModel` cross-project
fixture got the same treatment for both source and target mocks.
`mock.close.assert_called_once()` -> `mock.__exit__.assert_called_once()`
across 5 sites (the cleanup is now invoked via __exit__, not close).

Verification:
- `make check` clean: 3613 passed, 7 skipped, 107 deselected.
- All 4 e2e tests against project 1143 still pass in 32.91s:
  storage create-table --if-not-exists round-trip, semantic-layer
  search-context + get-context, sync diff --branch, fresh-CREATE
  writeback + KBC.* propagation.
- `ty check` clean.

Net diff: -55 LOC in semantic_layer_service.py (now well under the
1500-LOC ceiling at ~1441) thanks to losing the explicit
finally/close at every site.
Copy link
Copy Markdown
Contributor Author

@ottomansky ottomansky 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 #348 — feat(0.47.0): fresh-CREATE writeback + semantic-layer reads + sync/storage ergonomics

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, not duplicated here.

Summary

This PR bundles three coherent changes for v0.47.0: (A) two new semantic-layer search-context and get-context read commands extracted into _semantic_layer_lookup.py, (B) sync-push fresh-CREATE manifest writeback that updates placeholder entries in-place and propagates KBC.configuration.* metadata, and (C) ergonomic flags (--branch for sync push/pull/diff, --if-not-exists for storage create-table, --no-name-drift-warnings). The iteration-6 with-refactor focus was verified exhaustively: all 20 single-client sites and the 1 dual-client promote site in semantic_layer_service.py are correctly converted to context-manager blocks; _semantic_layer_lookup.py uses with open_client() as client: at both call sites; no residual try/finally + .close() exists in those files; the dual-client promote uses the Python 3.10+ parenthesized with (A as a, B as b): form; both clients in promote are correctly closed when an exception fires mid-body (verified by TestPromoteModel.test_both_clients_closed_even_on_error); and make check passes clean at 3613 passed.

Verdict: COMMENT — zero blocking findings. Three non-blocking items worth fixing before or shortly after merge.

Verdict

  • Verdict: COMMENT
  • Blocking findings: 0
  • Non-blocking findings: 3
  • Nits: 2

Blocking findings

(none)

Non-blocking findings

[NB-1] src/keboola_agent_cli/services/semantic_layer_service.py:214,238,1238 + tests/test_semantic_layer_service.py:2594 — four stale "try/finally" / "close()" docstrings survive the with-refactor

Four docstrings still describe the pre-refactor resource-management pattern after the iteration-6 conversion:

  • semantic_layer_service.py:214 (class docstring): "Cross-project operations (promote) hold two clients in a try/finally and close both even on failure."
  • semantic_layer_service.py:238 (_new_metastore_client): "Caller is responsible for close()."
  • semantic_layer_service.py:1238 (promote_model): "Holds two metastore clients in a try/finally and closes both on exit even on error."
  • tests/test_semantic_layer_service.py:2594: """try/finally must close the client even when list_items raises."""

The code is correct; only the documentation is stale. The risk is that a future contributor reading _new_metastore_client will add a bare client = self._new_metastore_client(project); ...; client.close() call instead of a with block, reasoning from the docstring. Fix: update the class docstring at line 214 to say "with self._new_metastore_client(project) as client:"; update line 238 to "Use as a context manager via with self._new_metastore_client(project) as client:."; update lines 1238-1239 to "Holds two metastore clients in a parenthesized with (...) as ... and closes both on exit via __exit__."; update the test docstring at 2594 to "with block must call __exit__ even when list_items raises."

[NB-2] tests/test_semantic_layer_cli.py:1399TestPermissions has no test asserting search-context and get-context pass through --deny-writes

TestPermissions.test_read_subcommand_allowed_with_deny_writes (line 1487) exercises semantic-layer show, but there is no equivalent test for the two new 0.47.0 read commands. Both are registered as "read" in OPERATION_REGISTRY (confirmed at permissions.py:177-178), so the permission engine will allow them — but there is no test asserting this property. If the registration were accidentally changed to "write" in a future PR, no unit test would catch the regression. Fix: add two tests in TestPermissions (or TestSearchContext/TestGetContext in the CLI file) asserting that --deny-writes does not block search-context and get-context.

[NB-3] tests/test_sync_service.pysync pull --branch is not directly covered by a unit test

The PR adds branch_override to SyncService.pull (line 284) and includes explicit unit tests for push (test_push_branch_override_reaches_client, line 3238) and diff (test_diff_branch_override_reaches_client, line 3268), but there is no analogous test_pull_branch_override_reaches_client. The pull branch resolver path shares the same mechanism as push/diff, so the omission is low-risk, but the test matrix is asymmetric: a reviewer would expect all three commands to have the same direct coverage. Fix: add a test mirroring test_push_branch_override_reaches_client but calling service.pull(...) with branch_override=999 and asserting the client's branch-filtering argument receives 999.

Nits

  • [NIT-1] pyproject.toml (PR title convention) — the conventional-commit scope (0.47.0) is a version number, which is non-standard (scopes normally name an affected area, e.g. (semantic-layer) or (sync)). The project's git log --oneline main shows scopes like (deps), (adr), (skill), (docs). Using a version as scope is not wrong, but it makes git log --oneline --grep scope: by area less useful. No action required if the version-as-scope is an intentional house convention; worth noting for consistency.

  • [NIT-2] src/keboola_agent_cli/services/_semantic_layer_lookup.py:107run_search_context passes items_by_type.get(item_type, []) without a model_uuid to client.list_items(wire_type), which intentionally returns all entities of that type across every model in the project. This is correct (it mirrors MCP search_semantic_context semantics) and documented in gotchas.md:92. However, results from different models are interleaved in the contexts[] array without a top-level model_uuid discriminator — the model identity is buried inside attributes.modelUUID on each item. The gotcha entry covers this, but a brief comment in _semantic_layer_lookup.py near line 107 noting "project-wide: no model_uuid filter, consumers must read attributes.modelUUID per item" would make the intent self-documenting at the call site.

Verification log

  • gh pr view 348 --json title,body,files → 29 files, +2786/-174, conventional feat(0.47.0): ✓, state OPEN ✓
  • git rev-parse --abbrev-ref HEADfeat/fresh-create-completeness matches PR <branch> field ✓
  • CONTRIBUTING.md Plugin synchronization map — all "NO" rows checked:
    • context.py AGENT_CONTEXT: search-context, get-context, --if-not-exists, --branch, --no-name-drift-warnings entries verified ✓
    • CLAUDE.md ## All CLI Commands: new command signatures present at lines 349, 435-436 ✓
    • keboola-expert.md §2 Tool Selection Matrix: search-context/get-context added to "List models / metrics" row; §1 Rule 6 VERSION GATE updated with 0.47.0 entry at line 113 ✓
    • commands-reference.md: new bullets for search-context, get-context, sync --branch, --if-not-exists, --no-name-drift-warnings present ✓
    • gotchas.md: fresh-CREATE writeback + 4 new entries tagged (since v0.47.0)
    • permissions.py OPERATION_REGISTRY: "semantic-layer.search-context": "read" and "semantic-layer.get-context": "read" present at lines 177-178 ✓
    • hints/definitions/semantic_layer.py: search-context hint at line 165, get-context hint at line 204 ✓
    • hints/definitions/storage.py: if_not_exists parameter wired at line 235 ✓
    • server/routers/semantic_layer.py: GET /search-context and GET /get-context routes at lines 259-284 ✓
    • server/routers/storage.py: if_not_exists field in request body + forwarded at lines 34/241 ✓
  • Layer violation scan (typer in services, httpx in commands, formatter in clients) → empty ✓
  • Magic numbers check → empty ✓
  • Raw error_code= string literals → empty ✓
  • Bare except: → empty ✓
  • print() in production code → empty ✓
  • Token in logged output without mask_token → clean ✓
  • make check → 3613 passed, 7 skipped, 107 deselected, 0 failed ✓
  • make typecheck → exit 0; 1 pre-existing unresolved-import warning on scripts/hatch_build.py (not introduced by this PR) ✓
  • with-refactor convergence check:
    • semantic_layer_service.py: 20 with self._new_metastore_client(project) as client: blocks, 0 raw .close() calls, 0 try/finally blocks (ignoring docstrings) ✓
    • promote_model site: parenthesized with (src as A, tgt as B): at line 1257 ✓
    • _semantic_layer_lookup.py: 2 with open_client() as client: blocks, 0 raw .close() calls ✓
    • MetastoreClient.__enter__ returns self, __exit__ calls self.close() (lines 77-81) ✓
    • Test fixture _make_service: mock.__enter__ = MagicMock(return_value=mock) + mock.__exit__ = MagicMock(return_value=False)
    • 5 mock.__exit__.assert_called_once() assertion sites (lines 266, 2028, 2029, 2604, 2682) ✓
    • Stale docstrings at semantic_layer_service.py:214,238,1238 and tests/test_semantic_layer_service.py:2594 — code correct, docs wrong (NB-1) ✓
    • LOC count: wc -l semantic_layer_service.py → 1432, under the 1500 hard ceiling ✓
  • fnmatch.fnmatchcase("", "*")True — empty-name items match the default * pattern and appear in search_context results; acceptable (mirrors MCP behavior, attributes included for caller filtering) ✓
  • Behavior reproduction: could not run live kbagent semantic-layer search-context (no E2E credentials in this session). PR provides live validation receipts against project 1143 with exact output. E2E tests in TestE2E_0_47_0_NewSurfaces cover all four new surfaces. Accepted as authoritative ✓

Open questions for the author

(none)

@ottomansky ottomansky marked this pull request as ready for review May 26, 2026 08:49
GitHub CI's `make check` runs `pytest -m "not e2e"` and the
`TestE2E_0_47_0_NewSurfaces` class was correctly tagged with
`@pytest.mark.e2e`, but I missed the second decorator the other E2E
classes in this file use: `@skip_without_credentials`. Without it,
when CI somehow does collect the class (e.g. via `pytest tests/`
without the `-m "not e2e"` filter, or via another wrapper), the
fixture tries to read `os.environ[ENV_TOKEN]` and raises `KeyError`
during setup rather than skipping cleanly.

Reproducer:
  unset E2E_API_TOKEN; uv run pytest tests/test_e2e.py::TestE2E_0_47_0_NewSurfaces
  -> 4 ERROR ... KeyError: 'E2E_API_TOKEN'

After the fix:
  unset E2E_API_TOKEN; uv run pytest tests/test_e2e.py::TestE2E_0_47_0_NewSurfaces
  -> 4 SKIPPED in 0.20s

This matches the pattern every other E2E class in the file uses
(see TestFullE2E, TestE2EErrorHandling, TestE2EJsonConsistency,
TestE2ESyncWorkflow -- all stack `@skip_without_credentials` ABOVE
`@pytest.mark.e2e`).

Verified GitHub Actions log for run 26441694904 showed exactly this
failure mode: "ERROR ... KeyError: 'E2E_API_TOKEN'" on all four new
tests, with 3610 non-e2e tests passing alongside.
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.

1 participant