fix(cds-caching): atomic UPSERT in KeyvCDS.set to stop PK-collision error noise (#3102)

CLAUDE.md

@@ -152,7 +152,7 @@ The load-bearing few. **Full detail for every relocated item → [tutorials-ims-
 - **`@cap-js/ai` for RPT-1 ValueList recommendations (#959)** — `AICore-mocked` locally, `AICore-btp` in hybrid/prod; per-field opt-out `@UI.RecommendationState: 0`. Ref: [cap-ai-plugin.md](docs/developers/reference/cap-ai-plugin.md).
 - **Knowledge-graph feature flags (`KG_*`), all default OFF + DEV-only, all fail-open** — PageRank #916, WCC isolation #918, on-demand extraction #948, Louvain communities #917, orphan retirement #1115, community peers/labels #1126, community search weight #1171, coverage nudge #1172, cluster Q&A #1173. Toggles, nightly jobs, and fail-open specifics → gotchas.md "Knowledge graph feature flags".
 - **HCQL protocol adapter (#2247, CAP 10.1.0)** — `POST /hcql/<svc>` paths (NOT co-mounted on OData URLs) on 5 authenticated services only (`AdminService`, `AuthorService`, `AnalyticsService`, `ExportsService`, `ConsolidationService`); requires `@sap/cds >= 10.1.0`. Malformed-CQN process-exit DoS from 10.0.3 **fixed in 10.1.0** (now returns 400, server stays up). Auth enforced before CQN parse. Kill: delete `srv/hcql-enablement.cds` + drop the `hcql` entry from `AdminService`'s `@protocol` list in `srv/admin-service-mcp.cds`, then `cds build --production` + redeploy. Ref: [hcql-support.md](docs/developers/reference/hcql-support.md).
-- **cds-caching CDS-DB store + metrics ON (#1222)** — `store:"cds"` in hybrid/prod, `memory` in base/unit. CF resolve-guard needs BOTH the baked csn entities AND `srv/lib/strip-precompiled-plugin-roots.js` (both load-bearing). Ref: [cds-caching-store.md](docs/developers/reference/cds-caching-store.md).
+- **cds-caching CDS-DB store + metrics ON (#1222)** — `store:"cds"` in hybrid/prod, `memory` in base/unit. CF resolve-guard needs BOTH the baked csn entities AND `srv/lib/strip-precompiled-plugin-roots.js` (both load-bearing). A committed `patch-package` patch (`patches/cds-caching+2.1.0.patch`, `postinstall`) makes `KeyvCDS.set` an atomic UPSERT to kill PROD PK-collision error noise on concurrent `kg:rl:*` writes (#3102); **re-verify the patch on any `cds-caching` bump** (patch-package fails install loudly on drift). Ref: [cds-caching-store.md](docs/developers/reference/cds-caching-store.md).
 - **User-facing UI changes want a committed e2e spec** — advisory PR nudge on `app/**`/`hugo/**` changes; real coverage runs in the post-DEV-deploy `e2e` job. Ref: [e2e-coverage-pattern.md](docs/developers/reference/e2e-coverage-pattern.md).
 - **`test:e2e` is post-deploy only, not on PRs** — self-skips without `SMOKE_BASE_URL`. Served tutorials render `<main>`+`<h1>`, NOT `<article>`. Runbook: `test/e2e/README.md`.
 - **Freshness detector grounding needs the corpus-embedding backfill** — until `srv/jobs/freshness-corpus-embedding-job.js` runs, every API-obsolescence claim degrades to `confidence: Low`. Tutorial source from `ContentFiles.sourceContent` via `getTutorialSource(slug)`, NOT `Steps.description`. → gotchas.md "Freshness detector".

docs/developers/reference/cds-caching-store.md

@@ -356,3 +356,62 @@ command-palette concept search): `@cache: { ttl: 300000, tags: [{ value:
   re-enabled metrics actually accumulate against the deployed DEV container (clear
   the stale zeroed rows first), then let PROD traffic post-cutover give the
   baseline before annotating more surfaces.
+
+## `KeyvCDS.set` UPSERT patch (#3102)
+
+**Symptom (PROD, 2026-11-11):** recurring `level:"error"` lines from the
+`cds-caching` logger:
+
+```
+KeyvCDS set error for key kg:rl:api:auth:auth:<hash>: unique constraint violated:
+Table(PLUGIN_CDS_CACHING_CACHESTORE), Index(_SYS_TREE_CS_#..._#P0) ... key: ID=kg:rl:...
+```
+
+Two namespaces seen — both the KG rate-limiter (`srv/lib/rate-limit/shared-limiter.js`,
+key prefix `kg:rl:`): `kg:rl:api:auth:auth:<hash>` and `kg:rl:homepage:anon:<ip>`.
+
+**Root cause:** the plugin's CDS-DB store adapter,
+`node_modules/cds-caching/lib/support/KeyvCDS.js`, implemented `set()` as a
+**read-then-create/update** (TOCTOU):
+
+```js
+const existing = await db.read(entity, key);
+if (existing) { await db.update(entity, key).with({ value, expiresAt }); }
+else          { await db.create(entity, { ID: key, value, expiresAt }); }
+```
+
+Under multi-instance (HA) CF, two instances both read "no row" for the same
+`kg:rl:*` counter, both take the `else` branch, and the second `create` violates
+the `CacheStore` PK unique index. The adapter's own `catch` logs at **error**
+level and swallows the exception — so the collision never reaches our
+`shared-limiter.js` fail-open (the limiter already over-allows under races, which
+is acceptable), but it spams PROD error monitoring and the losing write is
+dropped.
+
+**Fix:** a committed **patch-package** patch,
+`patches/cds-caching+2.1.0.patch`, replaces that branch with a single atomic
+UPSERT:
+
+```js
+await db.run(cds.ql.UPSERT.into(entity).entries({ ID: key, value, expiresAt }));
+```
+
+`CacheStore` has a single-column PK (`ID : String(900)`), so UPSERT compiles to
+HANA `UPSERT ... WITH PRIMARY KEY` and to SQLite `INSERT OR REPLACE` —
+last-write-wins, no read-then-write race, no unique-constraint collision. The
+patch re-applies on every `npm ci` / `npm install` via the `postinstall` hook
+(`patch-package`). Regression test:
+`srv/lib/__tests__/keyv-cds-upsert.test.js` (drives the real `KeyvCDS` against an
+in-memory CAP fixture, modelling the TOCTOU window deterministically).
+
+**Upstream:** the same read-then-insert is still present on `cds-caching` `main`.
+Filed upstream as
+[mikezaschka/cds-caching#44](https://github.com/mikezaschka/cds-caching/issues/44)
+(issue) +
+[mikezaschka/cds-caching#45](https://github.com/mikezaschka/cds-caching/pull/45)
+(PR). **Drop condition:** once a published `cds-caching` release carries the
+upstream fix and we adopt it, delete `patches/cds-caching+2.1.0.patch`.
+**Re-verify the patch on any `cds-caching` version bump** — `patch-package`
+fails the install loudly if the target file drifted, so the drift surfaces at
+`npm ci`, not silently.
+

docs/superpowers/plans/2026-11-11-cds-caching-upsert-fix.md

@@ -0,0 +1,231 @@
+# cds-caching UPSERT Fix (#3102) Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Stop `cds-caching` from logging `error`-level unique-constraint-violation noise in PROD when two HA instances concurrently write the same cache key (notably `kg:rl:*` rate-limiter counters), by making the `KeyvCDS.set()` write path an atomic single-key UPSERT.
+
+**Architecture:** The defect lives in the vendored `cds-caching` plugin (`node_modules/cds-caching/lib/support/KeyvCDS.js`), whose `set()` does read-then-`create`/`update` — a TOCTOU race that collides on the `CacheStore` PK under multi-instance CF. We fix it two ways: (1) **locally and immediately** via a committed `patch-package` patch against our pinned `cds-caching@2.1.0`, replacing the racy branch with `UPSERT.into(entity).entries(...)` (last-write-wins, no collision); (2) **upstream** by filing an issue + PR against `mikezaschka/cds-caching` so the patch can eventually be dropped. The bug is confirmed still present on upstream `main`.
+
+**Tech Stack:** Node.js 22, `@sap/cds` (CQL `UPSERT`), `cds-caching@2.1.0`, `patch-package`, HANA (prod/hybrid) / SQLite (unit).
+
+**Spec:** GitHub issue [sap-tutorials/tutorials-ims#3102](https://github.com/sap-tutorials/tutorials-ims/issues/3102)
+
+## Global Constraints
+
+- `cds-caching` is pinned at exactly `"2.1.0"` in `package.json` dependencies — the patch filename and `patch-package` match this version verbatim; do **not** bump it as part of this fix.
+- Node engine is pinned `22.23.4`; `patch-package` must run under it.
+- No raw SQL — use CQL `UPSERT` (`@sap/cds`), consistent with the CAP constraint in CLAUDE.md.
+- The `CacheStore` entity is single-PK: `key ID : String(900); value : LargeString; expiresAt : Int64;`. UPSERT semantics are only safe here because the key is a single PK column (composite-key UPSERT on HANA via CDS QL is a known gotcha — not applicable).
+- The fix must remain **fail-open**: `set()` must still never throw to its caller (keep the existing `try/catch` returning normally).
+- `patch-package` must re-apply automatically on `npm ci` / `npm install` (CI uses `npm ci`), via a `postinstall` hook, without breaking the existing `prepare` script.
+- The MCP MTA (`tutorials-mcp`) and QA app reuse the same `srv/lib` tree but **not** `node_modules` patching concerns — no `srv-qa` cp-list change is needed (no new `srv/lib/` file).
+
+## Review Focus
+
+- **npm script collision:** the repo has `prepare` (git hooks) but no `postinstall`. Adding `postinstall: patch-package` must not clobber `prepare`; both must run on install. — covered in Task 2 (verify `npm ci` output shows both hooks firing).
+- **Patch fails to apply after a future `cds-caching` bump:** `patch-package` errors (non-zero) if the target file drifted. Task 2 pins the exact `2.1.0` and documents the drop-on-upgrade path; the drift surfaces loudly at install, not silently.
+- **UPSERT on SQLite (unit profile):** unit tests use in-memory SQLite with `store:"memory"` (not `cds`), so KeyvCDS is never exercised there; the patch's UPSERT path is only hit under `store:"cds"`. Task 1 adds a direct KeyvCDS test against an in-memory SQLite db to prove UPSERT works on *both* dialects and that a double-set of the same key does not throw.
+- **`value`/`expiresAt` null handling:** a `set` with no TTL stores `expiresAt: null`; UPSERT must preserve that (not drop the column). Task 1 asserts a re-set with a shorter/absent TTL overwrites cleanly.
+- **Concurrent set does not lose the row entirely:** after two racing sets, the key must still be readable (one winner), never left absent. Task 1 asserts the key exists post-double-set.
+
+---
+
+### Task 1: patch-package patch — atomic UPSERT in `KeyvCDS.set()` (+ regression test)
+
+**Files:**
+- Create: `patches/cds-caching+2.1.0.patch` (generated by `patch-package`, not hand-written)
+- Modify (as patch source): `node_modules/cds-caching/lib/support/KeyvCDS.js` — the `set(key, value, ttl)` method body
+- Test: `srv/lib/__tests__/keyv-cds-upsert.test.js`
+
+**Interfaces:**
+- Consumes: nothing from earlier tasks.
+- Produces: a committed `patches/cds-caching+2.1.0.patch` that Task 2 wires into install; the behavioral guarantee that `KeyvCDS.set()` is collision-free and still never throws.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `srv/lib/__tests__/keyv-cds-upsert.test.js`. It instantiates the real `KeyvCDS` class from `cds-caching`, backed by an in-memory SQLite `cds.db` with the `plugin.cds_caching.CacheStore` entity deployed, and asserts the collision-safe contract:
+
+```js
+import { describe, it, expect, beforeAll } from 'vitest';
+import cds from '@sap/cds';
+import KeyvCDS from 'cds-caching/lib/support/KeyvCDS.js';
+
+describe('KeyvCDS.set is collision-safe (issue #3102)', () => {
+  beforeAll(async () => {
+    // deploy only the cache-store entity into an in-memory sqlite db
+    cds.model = await cds.load([require.resolve('cds-caching/db/cache-store.cds')]);
+    await cds.deploy(cds.model).to('sqlite::memory:');
+  });
+
+  it('a second set of the same key does NOT throw and overwrites (last-write-wins)', async () => {
+    const store = new KeyvCDS({});
+    await store.set('dup', { count: 1 }, 10_000);
+    await expect(store.set('dup', { count: 2 }, 10_000)).resolves.not.toThrow();
+    expect(await store.get('dup')).toEqual({ count: 2 });
+  });
+
+  it('two concurrent sets of a fresh key both resolve and the key is present', async () => {
+    const store = new KeyvCDS({});
+    await Promise.all([store.set('race', { a: 1 }, 10_000), store.set('race', { a: 2 }, 10_000)]);
+    expect(await store.get('race')).toBeDefined();
+  });
+
+  it('re-set with no TTL overwrites cleanly (expiresAt null)', async () => {
+    const store = new KeyvCDS({});
+    await store.set('nt', 'first', 10_000);
+    await store.set('nt', 'second'); // no ttl
+    expect(await store.get('nt')).toBe('second');
+  });
+});
+```
+
+The decisive assertion is that a simulated collision does not surface. (On SQLite a true PK re-INSERT would throw `UNIQUE constraint failed`; the pre-patch read-then-create code path can hit it under interleaving. Post-patch UPSERT never does.)
+
+- [ ] **Step 2: Run the test against the UNPATCHED plugin to see it is not yet collision-proof**
+
+Run: `npm test -- srv/lib/__tests__/keyv-cds-upsert.test.js`
+Expected: the concurrent-set / double-set assertions fail or emit the `KeyvCDS set error` log (pre-patch read-then-create path). If the unpatched code happens to pass serially, note it — the patch's value is proven by the HANA PK semantics; keep the test as the post-patch regression guard.
+
+- [ ] **Step 3: Edit `node_modules/cds-caching/lib/support/KeyvCDS.js`**
+
+Replace the body of `async set(key, value, ttl)` so the read-then-`create`/`update` branch becomes a single atomic UPSERT, keeping the outer `try/catch` (fail-open, no re-throw) intact:
+
+```js
+async set(key, value, ttl) {
+    try {
+        const { db, entity } = await this._getDB();
+        if (!db || !entity) return;
+        const expiresAt = ttl ? Date.now() + ttl : null;
+        // Atomic last-write-wins. CacheStore has a single-column PK (ID),
+        // so UPSERT compiles to HANA `UPSERT ... WITH PRIMARY KEY` and to
+        // SQLite `INSERT OR REPLACE` — no read-then-write race, no unique
+        // constraint collision under multi-instance concurrency (#3102).
+        await db.run(UPSERT.into(entity).entries({ ID: key, value, expiresAt }));
+    } catch (error) {
+        this.log.error(`KeyvCDS set error for key ${key}:`, error);
+    }
+}
+```
+
+`UPSERT` is a CQL global provided by `@sap/cds`; `cds` is already required at the top of the file (`const cds = require('@sap/cds')`), and `UPSERT` is exposed as a global by the cds runtime. If `UPSERT` is not in scope in this module, reference it as `cds.ql.UPSERT`. Confirm which form resolves before generating the patch.
+
+- [ ] **Step 4: Run the test to verify it passes against the patched plugin**
+
+Run: `npm test -- srv/lib/__tests__/keyv-cds-upsert.test.js`
+Expected: PASS, no `KeyvCDS set error` log emitted.
+
+- [ ] **Step 5: Generate the patch file**
+
+Run: `npx patch-package cds-caching`
+Expected: creates `patches/cds-caching+2.1.0.patch` containing only the `set()` diff. Inspect it to confirm it touches exactly that method and nothing else.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add patches/cds-caching+2.1.0.patch srv/lib/__tests__/keyv-cds-upsert.test.js
+git commit -m "fix(cds-caching): atomic UPSERT in KeyvCDS.set to stop PK-collision error noise (#3102)"
+```
+
+---
+
+### Task 2: Wire `patch-package` into install
+
+**Files:**
+- Modify: `package.json` — add `patch-package` to `devDependencies`, add/extend a `postinstall` script.
+
+**Interfaces:**
+- Consumes: `patches/cds-caching+2.1.0.patch` from Task 1.
+- Produces: automatic patch re-application on `npm ci` / `npm install`.
+
+- [ ] **Step 1: Add the dev dependency**
+
+Run: `npm install --save-dev patch-package`
+Expected: `patch-package` appears in `devDependencies`. (It also pulls its own deps; confirm the install succeeds under Node 22.)
+
+- [ ] **Step 2: Add the `postinstall` hook**
+
+In `package.json` `scripts`, add:
+```json
+"postinstall": "patch-package"
+```
+Leave the existing `prepare` script untouched. (npm runs `postinstall` then `prepare` on a bare `npm install`; both fire.)
+
+- [ ] **Step 3: Verify the patch applies from a clean install**
+
+Run: `npm ci`
+Expected: install output includes a `patch-package` line applying `cds-caching@2.1.0` cleanly; `node_modules/cds-caching/lib/support/KeyvCDS.js` contains the UPSERT edit afterward. Confirm with: `grep -c "UPSERT.into" node_modules/cds-caching/lib/support/KeyvCDS.js` → `1`.
+
+- [ ] **Step 4: Re-run the regression test post-`npm ci`**
+
+Run: `npm test -- srv/lib/__tests__/keyv-cds-upsert.test.js`
+Expected: PASS (proves the patch survives a real install cycle).
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add package.json package-lock.json
+git commit -m "build: wire patch-package postinstall for cds-caching UPSERT patch (#3102)"
+```
+
+---
+
+### Task 3: Document the patch + upgrade path
+
+**Files:**
+- Modify: `docs/developers/reference/cds-caching-store.md` — add a short "KeyvCDS.set UPSERT patch (#3102)" section.
+- Modify: `CLAUDE.md` — one line in the cds-caching gotcha bullet noting the committed patch and that it must be re-verified on any `cds-caching` bump.
+
+**Interfaces:**
+- Consumes: the patch + wiring from Tasks 1–2.
+- Produces: operator-facing record of why the patch exists and how to drop it.
+
+- [ ] **Step 1: Add the reference-doc section**
+
+Append to `docs/developers/reference/cds-caching-store.md` a section explaining: the PROD symptom (`error`-level `unique constraint violated` on `PLUGIN_CDS_CACHING_CACHESTORE` from concurrent `kg:rl:*` writes), the root cause (read-then-insert TOCTOU in `KeyvCDS.set`), the fix (committed `patches/cds-caching+2.1.0.patch` → atomic UPSERT), the upstream issue/PR links (Task 4), and the drop condition (patch becomes unnecessary once a `cds-caching` release carrying the upstream fix is adopted — re-verify/remove the patch on any version bump).
+
+- [ ] **Step 2: Add the CLAUDE.md note**
+
+Extend the existing `cds-caching CDS-DB store` gotcha bullet with: a committed `patch-package` patch (`patches/cds-caching+2.1.0.patch`) makes `KeyvCDS.set` an atomic UPSERT to kill PROD PK-collision error noise (#3102); re-verify the patch on any `cds-caching` upgrade.
+
+- [ ] **Step 3: Commit**
+
+```bash
+git add docs/developers/reference/cds-caching-store.md CLAUDE.md
+git commit -m "docs(cds-caching): record KeyvCDS.set UPSERT patch + upgrade path (#3102)"
+```
+
+---
+
+### Task 4: Upstream the fix (issue + PR to mikezaschka/cds-caching)
+
+**Files:** none in this repo (external repo). This task produces links recorded in Task 3's docs — run Task 4 before Task 3 Step 1 if you want the links inline, or update the doc afterward.
+
+**Interfaces:**
+- Consumes: the diagnosis + UPSERT diff from Task 1 (the upstream patch is the same edit applied to `main`, where `set()` is byte-identical).
+- Produces: an upstream issue URL and PR URL.
+
+- [ ] **Step 1: File the upstream issue**
+
+Against `mikezaschka/cds-caching`: title ≈ "KeyvCDS.set read-then-insert races on PK under multi-instance → unique-constraint errors". Body: the HA race, the `PLUGIN_CDS_CACHING_CACHESTORE` collision, that `set()` on `main` still does read-then-`create`/`update`, and the proposed atomic-UPSERT fix. Confirm no existing open issue covers it first.
+
+- [ ] **Step 2: Open the upstream PR**
+
+Fork `mikezaschka/cds-caching`, branch, apply the same `set()` UPSERT edit to `lib/support/KeyvCDS.js` on `main`, and open a PR referencing the issue. Match upstream code style (4-space indent, the existing comment tone). Include a one-line note that single-PK `CacheStore` makes UPSERT safe on HANA + SQLite + Postgres.
+
+- [ ] **Step 3: Record the links**
+
+Paste the issue + PR URLs into the Task 3 reference-doc section and the local #3102 PR description.
+
+---
+
+### Task 5: Open the tutorials-ims PR
+
+**Files:** none (git/PR operation).
+
+- [ ] **Step 1: Push the branch and open a DEV-targeted PR**
+
+Per repo convention, PRs target `DEV` (not `main`). Push `worktree-fix-3102-cds-caching-upsert` and `gh pr create --base DEV`. PR body: link #3102, summarize root cause + the two-pronged fix (local patch + upstream issue/PR), link the upstream URLs, and note the patch's drop condition.
+
+- [ ] **Step 2: Verify the full unit suite is green in CI**
+
+The full `npm test` suite is hook-blocked locally; confirm the CI unit job passes on the PR before requesting review.