fix(0.43.5): metastore duplicate-name accepts 409 alongside legacy 500

[ottomansky]

Problem

MetastoreClient.post_item has a workaround for the metastore's historical "duplicate name → HTTP 500 'Failed to create meta object'" quirk that normalises it to ErrorCode.ALREADY_EXISTS. keboola/go-monorepo#513 fixes the upstream bug so the metastore now returns a proper HTTP 409 Conflict with the message "Object with this name already exists in this project".

The current post_item workaround only matches status_code == 500 and a substring of the legacy message. Against a post-fix metastore deployment:

• 409 falls through to BaseHttpClient's generic error path → returns ErrorCode.API_ERROR with message "API error 409 from ...: Object with this name already exists ..." instead of the clean ALREADY_EXISTS path that command-layer error renderers and exit-code mapping rely on.
• Plus, 500 is in RETRYABLE_STATUS_CODES (constants.py:15) so today every duplicate-name POST eats MAX_RETRIES round-trips before the workaround even fires. 409 is not retryable, so once the metastore fix is live, this PR also tightens the retry cost from 3 round-trips down to 1.

Change

Two-line condition extension in metastore_client.py:post_item:

is_duplicate = exc.status_code == 409 or (
    exc.status_code == 500 and "Failed to create meta object" in exc.message
)

• 409 path: accept any 409 from POST /api/v1/repository/{type} (by construction a uniqueness violation; see services/metastore/api/handlers/repository_errors.go in the go-monorepo PR — the only constraint name the handler matches in the create path is ConstraintObjectTypeName).
• 500 path: retained, still gated on the substring so an unrelated 500 (DB outage, etc.) keeps surfacing as a retryable API_ERROR and doesn't get miscategorised as a name collision.

Backwards-compatible across the rollout window — works against stacks with the metastore fix and stacks without it.

PATCH on a missing UUID is also fixed upstream (returns 404 now), but BaseHttpClient._handle_error already maps 404 → ErrorCode.NOT_FOUND at http_base.py:249, so no client change is needed for that path.

Docstring on metastore_client.py updated to describe both server-side shapes.

Test plan

• uv run pytest tests/test_metastore_client.py -q → 14 passed (existing 13 + 1 new)
• make version-sync propagated 0.43.4 to plugin.json and marketplace.json
• New test_duplicate_name_409_becomes_already_exists registers a single 409 (asserts no retry), verifies status_code=409, error_code=ALREADY_EXISTS, retryable=False, and the canonical user-facing message
• Existing test_duplicate_name_500_becomes_already_exists + test_unrelated_500_passes_through stay green (legacy / unrelated paths unchanged)

Coordination

This PR is safe to merge before keboola/go-monorepo#513 lands — the 500 path is retained, so behaviour on stacks that haven't deployed the metastore fix is unchanged. After both PRs are merged + deployed, the retry-cost improvement also applies.

Notes

• make changelog-check flags Missing changelog entries for: 0.44.0b1 on upstream/main and on this branch identically. The 0.44.0b1 tag pre-exists without a changelog entry; not introduced by this PR.

[ottomansky]

Review of #318 — fix(0.43.4): metastore duplicate-name accepts 409 alongside legacy 500

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 extends MetastoreClient.post_item to recognise HTTP 409 Conflict (from
go-monorepo PR #513) as a duplicate-name signal alongside the legacy HTTP 500
workaround. The implementation is backward-compatible (500 path retained with the
original substring gate), the new 409 path correctly sets ErrorCode.ALREADY_EXISTS
with retryable=False, and no CLI command surface changes are introduced — so
OPERATION_REGISTRY, context.py, CLAUDE.md, and the hint definitions are
correctly untouched. One silent-drift finding: semantic-layer-workflow.md still
documents only the legacy 500 shape, so AI agents reading it will not know that
post-fix stacks emit 409 instead (they will still call kbagent semantic-layer add ...
correctly, but their mental model of the underlying contract will be stale).
No blocking issues were found.

Verdict: APPROVE — no blocking findings; one non-blocking silent-drift nit and
one cosmetic nit in the test.

Verdict

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

Blocking findings

(none)

Non-blocking findings

[NB-1] plugins/kbagent/skills/kbagent/references/semantic-layer-workflow.md:489 — stale "500" description needs updating to reflect 409

The file still reads:

- **Duplicate-name POST -> 500** with `"Failed to create meta object"`.
  kbagent normalizes to `ErrorCode.ALREADY_EXISTS`.

After this PR ships, the primary signal on post-fix stacks is a 409, not a 500. Any
AI agent reading this workflow reference will build a mental model of the protocol
that is wrong for the majority of stacks going forward. Per CONTRIBUTING.md the
Plugin synchronization map row for gotchas.md and topic workflow files should be
updated whenever client-visible behavior changes.

Suggested fix: update the bullet to reflect both shapes, e.g.

- **Duplicate-name POST -> 409 Conflict** (post go-monorepo PR #513) with
  `"Object with this name already exists in this project"`, or **500** with
  `"Failed to create meta object"` on legacy stacks. kbagent normalizes both
  into `ErrorCode.ALREADY_EXISTS` (since v0.43.4).

The (since v0.43.4) tag is required by CONTRIBUTING.md to prevent AI agents from
recommending the 409 behavior on stacks that haven't deployed the metastore fix yet
(though in this case the 409 behavior is transparent to callers — it's the server
that decides which code to return — so the operational risk is low; hence
NON-BLOCKING rather than BLOCKING).

Nits

• [NIT-1] plugins/kbagent/agents/keboola-expert.md:185 — the NEVER column still
  reads bypasses the duplicate-name 500-to-ALREADY_EXISTS normalization. Now that
  both 500 and 409 are normalized, 500-to-ALREADY_EXISTS is technically incomplete;
  duplicate-name-to-ALREADY_EXISTS normalization would be accurate going forward.
  Low-risk cosmetic; does not affect correctness of the advice (the NEVER guidance
  is still correct — raw metastore POSTs still bypass the normalization entirely).

• [NIT-2] tests/test_metastore_client.py:173-178 — the try/finally around
  pytest.raises is correct resource management, but the existing 500 tests at lines
  193-201 and 211-218 use the same pattern. Consider a @pytest.fixture for
  MetastoreClient(...) + client.close() to reduce boilerplate. Minor; consistency
  within the test class.

Verification log

• gh pr view 318 --json title,body,files --repo padak/keboola_agent_cli → 7 files,
  +56/-14, conventional fix: prefix ✓ (internal client behavior change, not
  feat:, which is correct per CONTRIBUTING.md)
• git rev-parse --abbrev-ref HEAD → fix/metastore-409-conflict-mapping ✓
• make check → ruff clean, format clean, SKILL.md up-to-date, plugin.json in sync;
  changelog-check exits 1 (Missing changelog entries for: 0.44.0b1) — confirmed
  pre-existing on upstream/main by checking out upstream changelog and running the
  same check; NOT introduced by this PR ✓
• uv run pytest tests/ -q --ignore=tests/test_e2e.py --ignore=tests/test_e2e_lineage_deep.py
  → 3394 passed, 26 skipped ✓
• uv run pytest tests/test_metastore_client.py -q → 14 passed (13 existing + 1
  new test_duplicate_name_409_becomes_already_exists) ✓
• 3-layer check: no typer/click imports added to services; no httpx calls in
  commands; no formatter calls in clients → empty grep results ✓
• Convention check: no bare except:, no raw error_code="..." strings, no
  print() in production code, no magic numbers added → empty grep results ✓
• Token discipline: only token=TOKEN test variable reference in diff; no real
  token pattern; mask_token() not needed (no error message surfaces the token) ✓
• Plugin synchronization map — no new CLI command registered; @*_app.command() grep
  on diff → empty ✓; OPERATION_REGISTRY, context.py, CLAUDE.md, hints/,
  commands-reference.md, keboola-expert.md §2 matrix correctly untouched ✓
• Silent-drift hunt: grep -rn 'Failed to create meta' . --include='*.md' →
  semantic-layer-workflow.md:489 still uses the legacy-only wording → NB-1 above
• keboola-expert.md line 185 NEVER column: references 500-to-ALREADY_EXISTS →
  NIT-1 above
• Backward-compat: 500 path retained with identical substring gate; unrelated-500
  test (test_unrelated_500_passes_through) still registers MAX_RETRIES responses
  → retryable API_ERROR behavior confirmed ✓
• 409 retryable=False contract: RETRYABLE_STATUS_CODES = {429, 500, 502, 503, 504} in constants.py:15 — 409 absent → no retries before normalization fires ✓
• Behavior reproduction: manual execution not attempted (no test metastore available);
  new test test_duplicate_name_409_becomes_already_exists registers exactly one 409
  response (proving no retry) and asserts retryable=False — coverage is adequate
  for the claim ✓
• Version bump: pyproject.toml 0.43.3 → 0.43.4; plugin.json + marketplace.json
  • uv.lock all bumped ✓; changelog entry present and descriptive ✓

Open questions for the author

(none)

[ottomansky]

Addressing review feedback

Pushed b920d51 covering NB-1 + NIT-1 + NIT-2:

• NB-1 — plugins/kbagent/skills/kbagent/references/semantic-layer-workflow.md:489 rewritten to describe both shapes:

  Duplicate-name POST → 409 Conflict (post go-monorepo PR Pin command-reference.md metavar format as a stable contract (docs gate depends on it) #513) with "Object with this name already exists in this project", or 500 with "Failed to create meta object" on legacy stacks. kbagent normalizes both into ErrorCode.ALREADY_EXISTS (since v0.43.4). 409 is non-retryable so the fix-deployed path avoids the MAX_RETRIES round-trips the 500 path still pays.

• NIT-1 — plugins/kbagent/agents/keboola-expert.md:185 NEVER column changed from 500-to-ALREADY_EXISTS to duplicate-name-to-ALREADY_EXISTS normalization (both 409 and legacy 500) so the wording stays accurate post-Pin command-reference.md metavar format as a stable contract (docs gate depends on it) #513.
• NIT-2 — tests/test_metastore_client.py TestDuplicateNameNormalization class now uses a module-scope metastore_client fixture, dropping the per-test try/finally. Scope intentionally narrowed to the class touched by this PR — the broader test file uses the inline pattern and refactoring it all is a separate cleanup.

No version bump — still 0.43.4, docs and test boilerplate only. uv run pytest tests/test_metastore_client.py -q → 14 passed.

Also ran a live e2e against the kbagent playground (e2e-1143 / project 1143, stack europe-west3.gcp):

1. Create model
2. Add glossary term (succeeds)
3. Re-add same term → server returns HTTP/1.1 500 Internal Server Error (the pre-fix metastore on that stack), kbagent retries 3× per RETRYABLE_STATUS_CODES, then the legacy-500-substring branch of the normalisation fires → {"code": "ALREADY_EXISTS", "retryable": false, ...} with exit code 1 ✓
4. Cleanup: term + model deleted, no residue

So backwards-compat is verified on a real metastore. Once go-monorepo #513 ships to this stack, the same e2e will hit the 409 branch (no retries) and surface the same ALREADY_EXISTS envelope.

[padak]

Review of #318 — fix(0.43.5): metastore duplicate-name accepts 409 alongside legacy 500

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 fixes a client-side normalization gap in MetastoreClient.post_item: after the upstream metastore fix (go-monorepo PR #513), the server now returns HTTP 409 Conflict for duplicate-name POSTs instead of the historical HTTP 500 quirk. The two-line condition extension is correct, well-justified, and backward-compatible during the rollout window. The version bump (0.43.4 -> 0.43.5), plugin-doc updates, new gotcha entry, and semantic-layer-workflow update are all present and correctly tagged with (since v0.43.5). The new test registers exactly one 409 response (asserting no retry), covers all key fields, and all 14 metastore tests plus the full 3396-test suite pass.

The single make check failure on changelog-check (Missing changelog entries for: 0.44.0b1) is pre-existing on main -- it predates this branch, is caused by a GitHub prerelease tag (v0.44.0b1) on feat/agent-cli-parity that has no entry in changelog.py, and is explicitly acknowledged in the PR description. This PR does not introduce or worsen it.

Verdict: APPROVE. No blocking findings.

Verdict

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

Blocking findings

(none)

Non-blocking findings

[NB-1] tests/test_metastore_client.py:231 — legacy 500 test missing status_code and retryable assertions

The new test_duplicate_name_409_becomes_already_exists (line 197) asserts status_code=409 and retryable=False. The existing test_duplicate_name_500_becomes_already_exists (line 217) asserts only error_code and message substrings, but does not assert status_code=500 or retryable=False. Since the PR preserves exc.status_code verbatim, the 500 path also sets retryable=False on the re-raised exception, but the test does not verify it. Adding the two assertions would lock in the contract symmetrically with the new 409 test -- a future refactor that accidentally made the 500 path retryable would otherwise go undetected.

Nits

• [NIT-1] plugins/kbagent/agents/keboola-expert.md:114 — the VERSION GATE list (§1 Rule 6) notes kbagent update --beta = 0.43.3+ as the most recent metastore-adjacent line. Since v0.43.5 introduces no new CLI commands and the 409 normalization is transparent to callers, a VERSION GATE entry is not required by the checklist. Purely informational: if the team ever wants to document "safe to call semantic-layer add against a post-PR#513 metastore stack" as a version gate, 0.43.5 is the anchor. Not a gap.

• [NIT-2] src/keboola_agent_cli/changelog.py:111 — the changelog entry for 0.43.5 is unusually long (the entire body is a single list element running to ~700 words). By convention, other entries in the file are one-liners or two-liners; the verbosity is useful for internal history but may cause kbagent changelog to render an awkward wall of text in human mode. Consider splitting into two shorter bullets, matching the 0.43.4 two-entry style already present in this PR's diff.

Verification log

• gh pr view 318 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → 10 files, +97/-31, state=OPEN, headRefName=fix/metastore-409-conflict-mapping, conventional fix(0.43.5): prefix matches a bug fix -- checked
• git rev-parse --abbrev-ref HEAD → fix/metastore-409-conflict-mapping -- working tree matches PR branch, proceeding
• grep -E '^\+.*@.*_app\.command\(' /tmp/kbagent-pr-318.diff → empty (no new CLI commands); Plugin synchronization map silent-drift surfaces skipped -- no command additions to track
• Layer violation checks (typer in services, httpx in commands, formatter in clients) → all empty -- no layer violations
• uv run pytest tests/test_metastore_client.py -v → 14 passed (existing 13 + 1 new test_duplicate_name_409_becomes_already_exists) -- exit 0
• uv run pytest tests/ -q --ignore=tests/test_e2e.py --ignore=tests/test_e2e_lineage_deep.py → 3396 passed, 26 skipped, 16 warnings -- exit 0
• make check → lint PASS, format PASS, SKILL.md PASS, plugin.json PASS, changelog-check FAIL (Missing changelog entries for: 0.44.0b1). Verified this failure also fires on main HEAD (ab4ba91) and at the merge-base -- pre-existing, introduced when gh release create v0.44.0b1 --prerelease was run against feat/agent-cli-parity without a corresponding changelog.py key. Not introduced by this PR.
• Convention checks (magic numbers, raw error_code strings, bare except, print() in production, token exposure) → all clean
• plugins/kbagent/skills/kbagent/references/gotchas.md:124 → new entry ## Metastore duplicate-name POST returns 409 OR 500 -- both map to ALREADY_EXISTS (since v0.43.5) -- version tag present and correct
• plugins/kbagent/skills/kbagent/references/semantic-layer-workflow.md:492 → since v0.43.5 tag present in the "Quick reminders" section
• plugins/kbagent/agents/keboola-expert.md diff → one-line update in the Tool Selection Matrix "Add a metric..." row: removes the stale "500-to-ALREADY_EXISTS" wording, replaces with generic "duplicate-name-to-ALREADY_EXISTS" -- correct
• E2E test coverage: no new CLI commands introduced, so no new E2E test is required per CONTRIBUTING.md -- the existing semantic-layer E2E tests cover the code path
• Behavior reproduction: could not run live against a post-go-monorepo-PR#513 metastore stack (no such stack available in the local environment). The unit test suite registers a single mock 409 and verifies the full normalized exception shape -- this is the appropriate substitute. Marked as unverified runtime behavior; needs author confirmation that the live 409 normalizes correctly against a deployed fix stack.

Open questions for the author

• The make check changelog-check failure exists identically on main (caused by the v0.44.0b1 GitHub prerelease tag on feat/agent-cli-parity having no changelog.py key). Is there a plan to add a "0.44.0b1" entry to changelog.py before that branch merges, or to configure generate_changelog.py to skip prerelease tags that live on non-main branches? The current state means every PR rebased on main will surface the same spurious CI failure.

[padak]

Review findings disposition (kbagent-pr-reviewer iter-1)

• NB-1 -- addressed in f0141eb. Two assertions backfilled on test_duplicate_name_500_becomes_already_exists (status_code=500, retryable=False). Symmetric coverage with the new 409 test.
• NIT-1 (VERSION GATE in keboola-expert.md) -- not adopted. keboola-expert.md is currently at 59995 / 60000 bytes after rebase onto main (5 B headroom); the reviewer explicitly flagged this as not required, and the gotcha entry in gotchas.md already carries the (since v0.43.5) anchor that AI agents will hit.
• NIT-2 (long single-list-item changelog entry) -- not adopted. The format matches every 0.43.0 -- 0.43.3 entry (each is one dense paragraph per release). Splitting just this release would create a stylistic inconsistency the renderer would surface as the anomaly instead.
• Pre-existing changelog-check failure (Missing changelog entries for: 0.44.0b1) -- not in scope for this fix. v0.44.0b1 lives on the feat/agent-cli-parity branch (PR feat(0.44.0): kbagent agent <verb> -- CLI parity for /agents REST surface #310) and will land its entry when the beta stabilises.

PR title also updated to fix(0.43.5): so it matches the bumped version.