#6228 Fix Agent Memory review findings

.ai/commands/pull-request-review.md

@@ -182,7 +182,7 @@ standards with good test coverage.
 1. **Code Style** (Style)
    - File: Multiple files
    - Issue: Inconsistent import ordering
-   - Recommendation: Run `yarn prettier:fix`
+   - Recommendation: Run `npm run prettier:fix`
 
 ## Risk Assessment
 

.ai/skills/code-quality/SKILL.md

@@ -15,7 +15,7 @@ description: >-
 
 ## Critical Rules
 
-- **ALWAYS run linter** after code changes: `yarn lint`
+- **ALWAYS run linter** after code changes: `npm run lint`
 - Linter must pass before committing
 - No console.log in production code (use console.warn/error only)
 
@@ -117,7 +117,7 @@ Also avoid:
 
 When updating npm packages (especially `@redis-ui/*` packages):
 
-1. **Clear Vite cache** after `yarn install`:
+1. **Clear Vite cache** after `npm install`:
 
    ```bash
    rm -rf node_modules/.vite
@@ -130,7 +130,7 @@ When updating npm packages (especially `@redis-ui/*` packages):
 
 ## Pre-Commit Checklist
 
-- [ ] `yarn lint` passes
+- [ ] `npm run lint` passes
 - [ ] No TypeScript errors
 - [ ] Import order is correct
 - [ ] No `any` types without reason

.ai/skills/dead-dependencies/SKILL.md

@@ -0,0 +1,134 @@
+---
+name: dead-dependencies
+description: >-
+  Find and safely remove unused ("dead") npm dependencies in RedisInsight
+  using a grep + leaf-check + build-gate recipe. Use when cleaning up
+  dependencies, investigating whether a package is still used, removing a
+  suspected leftover, or when the user mentions dead deps, unused
+  dependencies, dependency cleanup, leftover packages, or "is this safe to
+  remove". Complements the weekly vulnerability audit
+  (`scripts/dependency-audit-report.mjs`), which only reports vulnerabilities.
+---
+
+# Dead Dependencies
+
+Identify and safely remove unused npm dependencies. This is a **local,
+interactive** workflow — a scheduled report can only ever guess; confirming a
+dependency is dead requires grepping real usage and running the build.
+
+## Why manual (not knip/depcheck)
+
+Static tools over-report badly in this repo and must not be trusted blindly:
+
+- The root `package.json` is a **mega-manifest** (UI + desktop + build + test
+  + storybook + electron), so a tool scanning one area flags everything used
+  elsewhere as "unused".
+- Lots of dependencies are referenced **without an `import`**: webpack loaders
+  and eslint/jest/babel plugins by string in config, tools invoked from
+  `package.json` `scripts`, runtime `-r` preloads, dynamic `require`, and
+  ambient `@types/*`.
+- The webpack→Vite migration left **genuinely dead** build tooling behind,
+  mixed in with false positives.
+
+The reliable signal is: **grep for usage → confirm it's a leaf → remove it and
+run the build gate.** That is exactly how `jsonpath` was confirmed dead and
+removed (worked example at the bottom).
+
+## The recipe (per candidate)
+
+### 1. Grep the whole repo for real usage
+
+```bash
+PKG=jsonpath   # the dependency to check
+grep -rn "$PKG" \
+  redisinsight configs scripts tests .storybook \
+  --include=*.ts --include=*.tsx --include=*.js --include=*.jsx \
+  --include=*.mjs --include=*.cjs --include=*.json \
+  2>/dev/null | grep -v node_modules
+```
+
+Look for `import ... from '$PKG'`, `require('$PKG')`, `import('$PKG')`, and
+bare references. Ignore unrelated substring hits (e.g. `nestjs-form-data` when
+checking `form-data`, or a package name appearing only in tutorial/`manifest`
+text). **Zero real references → candidate for the next steps.**
+
+### 2. Rule out no-import usage
+
+A clean grep is **necessary but not sufficient**. Check the ways a package is
+used without an `import`:
+
+- **Config string references** — search the build/test config for the bare
+  name: `.eslintrc.js`, `configs/webpack.config.*.ts`,
+  `redisinsight/ui/vite.config.mjs`, `jest.config.cjs`, `babel.config.cjs`,
+  `electron-builder.json`, `.mocharc*`. eslint plugins, webpack loaders, and
+  jest/mocha reporters live here.
+- **`package.json` scripts** — a tool like `concurrently`, `lint-staged`, or a
+  reporter is "used" if a `scripts` entry (any workspace) invokes it.
+- **Runtime preloads / dynamic** — `node -r <pkg>`, `require(variable)`, or a
+  wasm/worker loader with a computed path.
+
+### 3. Leaf check
+
+```bash
+npm ls "$PKG"        # in the workspace that declares it
+```
+
+Confirm nothing else in the tree depends on it, and note any transitive deps it
+uniquely pulls (removing `jsonpath` also dropped `underscore`).
+
+### 4. Classify before acting
+
+| Situation | Action |
+| --- | --- |
+| Plain runtime/dev dep, zero references anywhere | **Delete** (after the gate below) |
+| `@types/x` where base `x` bundles its own types (`node_modules/x/package.json` has `types`/`typings`) | **Delete** — the DefinitelyTyped package is obsolete |
+| `@types/x` that's ambient/global-only (e.g. `@types/webpack-env`) | **Keep** — never imported by design |
+| Declared in more than one workspace (`package.json`), used in only one | **Relocate/dedupe** — remove the *unused* declaration, never the used copy |
+| Shadows a Node builtin (`buffer`, `assert`, …) | **Keep** — usually a false positive |
+| Referenced only in a config/`scripts` (step 2 hit) | **Keep** |
+
+### 5. The gate — the only real proof
+
+Remove it, reinstall (per the repo's dependency rules — never hand-edit
+`package.json`/lockfile, never `--ignore-scripts`), and verify the affected
+area builds:
+
+```bash
+npm uninstall "$PKG"          # in the declaring workspace; updates the lockfile
+npm run type-check            # or the affected workspace's type-check
+npm run test                  # / test:api, as relevant
+npm run build                 # if it's a build-time dep
+```
+
+Green across the relevant checks = safe. Commit the `package.json` +
+`package-lock.json` change (see the git-safety / dependency rules in
+`CLAUDE.md`). If anything goes red, it wasn't dead — restore it.
+
+## Optional: enumerate candidates to sweep
+
+To triage the whole surface rather than one package, list declared deps and
+grep each for an import, then apply steps 2–5 to the ones with zero hits:
+
+```bash
+node -e "const p=require('./package.json');console.log([...Object.keys(p.dependencies||{}),...Object.keys(p.devDependencies||{})].join('\n'))" \
+| while read PKG; do
+    hits=$(grep -rl --include=*.ts --include=*.tsx --include=*.js --include=*.jsx --include=*.mjs \
+      -e "from ['\"]$PKG" -e "require(['\"]$PKG" redisinsight configs scripts tests 2>/dev/null | grep -vc node_modules)
+    [ "$hits" = "0" ] && echo "candidate: $PKG"
+  done
+```
+
+This is a **first filter only** — every candidate still goes through steps 2–5.
+Expect false positives (config/string/dynamic/ambient use). Never delete straight
+from this list.
+
+## Worked example — `jsonpath`
+
+1. Grep across the repo → the only hits were unrelated tutorial text in a
+   `manifest.json`; **no `import`/`require`**.
+2. Not referenced in any config or script.
+3. `npm ls jsonpath` → a leaf; it was also the sole reason `underscore` was
+   installed.
+4. Plain runtime dep, zero references → likely deletable.
+5. Removed `jsonpath` + `@types/jsonpath`, `npm install`, `npm run type-check`
+   → green. Confirmed dead; committed. (`underscore` dropped with it.)

.ai/skills/e2e-testing/SKILL.md

@@ -94,7 +94,10 @@ npx playwright test                           # All projects
 
 Put a test in `tests/serial/` when it:
 - Shares database state across tests via `beforeAll`
-- Runs dangerous commands or mutates global app state
+- Runs dangerous commands or mutates global app state. Settings are global:
+  `scanThreshold`, `dateFormat`, `timezone` and `batchSize` are one shared resource,
+  so a spec that changes one and holds it across other work breaks every concurrent
+  test that reads it
 - Cannot tolerate concurrent execution with other tests
 - Would cause flakiness when run with other tests
 - Require special environment configuration
@@ -310,15 +313,36 @@ const config = ConfigFactory.build({ name: 'custom-name' });
 
 ### Cleanup Pattern
 
-Always prefix test data with `test-` for easy cleanup:
+Prefix test data with `test-` so leftovers are identifiable, but **never clean up by
+that prefix**. Specs share a Redis instance and different files run on different
+workers, so deleting every `test-*` key also removes keys the specs alongside it are
+still asserting on, and their rows disappear mid-test.
+
+Track what a test creates and delete only that:
 
 ```typescript
-// In apiHelper
-async deleteTestData(): Promise<number> {
-  return this.deleteByPattern(new RegExp(`^${TEST_PREFIX}`));
-}
+import { createKeyTracker } from 'e2eSrc/helpers';
+
+test.describe('Browser > Add Key', () => {
+  const keys = createKeyTracker();
+
+  test.afterEach(async ({ apiHelper }) => {
+    await keys.cleanup(apiHelper, database.id);
+  });
+
+  test('should add a Hash key', async ({ browserPage }) => {
+    const keyData = keys.track(HashKeyFactory.build());
+    // keys.add(name) records a name built without a factory, e.g. a rename target.
+  });
+});
 ```
 
+Miss a `track()` and the key is never deleted, which is harmless in CI (the test
+environment is recreated per run) but accumulates locally until the RTE restarts.
+
+Databases need no such care: each spec deletes its own in `afterAll`, and
+`browser.setup` clears leftover `test-` databases once before the run.
+
 ## Fixtures
 
 ### Add New Fixtures to base.ts

.ai/skills/i18n/SKILL.md

@@ -81,6 +81,29 @@ import { Trans } from 'uiSrc/i18n';
 // getTranslatedApiError() fills {{databaseId}} from response.data.resource — no extra code.
 ```
 
+## Plurals
+
+Use i18next's native **`count`-based** plurals — never a hand-rolled `isPlural` branch with
+`.single`/`.plural` keys.
+
+- Add one key per plural form with the i18next suffix: `key_one`, `key_other` (a language may
+  need more forms — `_few`, `_many` — but `en`/`bg` only use `_one`/`_other`).
+- Reference the **base** key (no suffix) and pass `count`; i18next selects the form:
+  `t('key', { count })` or `<Trans i18nKey="key" count={n} …/>`.
+- The base key type-checks even though only the suffixed forms are in `en.json` — i18next's
+  types resolve it from the `_one`/`_other` entries.
+- **Write the whole sentence in each form.** Don't interpolate the one differing word as a
+  fragment — word order, agreement, and the number of plural forms vary by language.
+- Renaming a key (e.g. `.single` → `_one`) leaves the old key behind in `bg.json` because
+  `i18n:extract` doesn't prune — delete the orphan so en/bg parity holds.
+
+```tsx
+// en.json:
+//   "workbench.runConfirm.body_one":   "…This command is part of…"
+//   "workbench.runConfirm.body_other": "…These commands are part of…"
+<Trans i18nKey="workbench.runConfirm.body" count={commands.length} components={{ bold }} />
+```
+
 ## Keys
 
 - **Flat, dotted keys** — `keySeparator` and `nsSeparator` are `false`, so a dot is a literal character, not nesting. `"api.error.code.11000.title"` is a single key.
@@ -130,13 +153,13 @@ The backend ships a stable `errorCode` on every user-facing error (see
 1. Add the key + English value to `en.json` **and** the same key to `bg.json` (translated, or empty to defer). Keep both sorted and in parity.
 2. Reference it: `t('my.key')` / `i18n.t('my.key')` / `<Trans i18nKey="my.key" …/>`.
 3. For dynamic values, pass `values` (interpolation) or `resource` (backend errors).
-4. Run `yarn i18n:extract` to sync/sort, and `yarn i18n:check` to catch duplicate keys.
-5. `yarn type-check` (new literal keys must resolve) and `yarn lint:ui`.
+4. Run `npm run i18n:extract` to sync/sort, and `npm run i18n:check` to catch duplicate keys.
+5. `npm run type-check` (new literal keys must resolve) and `npm run lint:ui`.
 
 ## Tooling
 
-- `yarn i18n:extract` — scans `t()`/`<Trans>` usages and syncs `en.json`/`bg.json` (alphabetical; does **not** prune unused keys). Note: dynamic (`as never`) keys aren't discovered by extraction — keep them in the locale files manually.
-- `yarn i18n:check` — fails if a locale file has a **duplicate key** (JSON silently keeps the last, so a dup would shadow a value). Runs in CI on PRs touching `locales/**`.
+- `npm run i18n:extract` — scans `t()`/`<Trans>` usages and syncs `en.json`/`bg.json` (alphabetical; does **not** prune unused keys). Note: dynamic (`as never`) keys aren't discovered by extraction — keep them in the locale files manually.
+- `npm run i18n:check` — fails if a locale file has a **duplicate key** (JSON silently keeps the last, so a dup would shadow a value). Runs in CI on PRs touching `locales/**`.
 - Dev override: append `?lang=bg` to the URL to preview Bulgarian.
 
 ## Do / Don't
@@ -146,3 +169,4 @@ The backend ships a stable `errorCode` on every user-facing error (see
 - ✅ Keep en/bg key parity; empty bg is an acceptable "later" placeholder.
 - ❌ Don't hardcode user-facing strings — add a key.
 - ❌ Don't hand-edit the locale-file key order — let `i18n:extract` sort.
+- ❌ Don't hand-roll plurals with a JS branch — use `count` + `key_one`/`key_other` (see Plurals).

.ai/skills/pull-requests/SKILL.md

@@ -10,10 +10,6 @@ description: >-
 
 ## Creating a PR
 
-### Labels
-
-When creating PRs with AI assistance, always add the **"AI-Made"** label.
-
 ### PR Title
 
 Include issue number at the start:
@@ -36,7 +32,7 @@ Describe how to test the changes.
 
 ---
 
-Closes #RI-123
+Refs #RI-123
 ```
 
 **PR Description Guidelines:**
@@ -45,6 +41,7 @@ Closes #RI-123
 - **Focus on high-level changes** - Don't list every code change in the #What section
 - **Brief and to the point** - The diff shows the details; describe the "why" and "what" at a high level
 - **Technical decisions** - Only mention significant architectural or design decisions if relevant
+- **Link, don't auto-close** - Use `Refs #RI-123` / `Addresses #RI-123`, not `Closes`/`Fixes`/`Resolves` - those keywords auto-close the issue when the PR merges, and tickets should be closed manually, not by the merge
 
 ## Review Process
 

.ai/skills/redis-insight-plugin/SKILL.md

@@ -33,7 +33,7 @@ See [references/official-docs-summary.md](references/official-docs-summary.md) f
 > - **[redis-ui-components](../redis-ui-components/)** — build all plugin UI from Redis UI
 >   components. Import the internal `uiSrc/components/ui` wrappers; **never** import raw
 >   `@redis-ui/*`. (This skill is a symlink into the installed `@redis-ui/components` package,
->   so it resolves after `yarn install`; if it is missing, run install — the canonical source is
+>   so it resolves after `npm install`; if it is missing, run install — the canonical source is
 >   `node_modules/@redis-ui/components/skills/redis-ui-components/`.)
 > - **[code-quality](../code-quality/SKILL.md)** — TypeScript everywhere (no `any`), naming
 >   (`PascalCase` / `camelCase` / `UPPER_SNAKE_CASE`), import order, no magic numbers, no
@@ -161,7 +161,7 @@ See [references/review-hardening.md](references/review-hardening.md).
 ## Build and Verify
 
 ```bash
-yarn build
+npm run build
 test -f dist/index.js
 test -f dist/styles.css                       # if "styles" is declared
 grep -c "process.env" dist/index.js           # must be 0 in a Parcel build

.ai/skills/redis-insight-plugin/references/error-handling.md

@@ -117,4 +117,4 @@ Validate persisted state before applying — fields can disappear or change shap
 | Values mapped to wrong field/axis | Response shape assumption wrong | Branch on the actual runtime shape, not the command name. |
 | Theme looks wrong | No `theme_DARK` handling | Read `document.body.classList` or `getTheme()` from SDK. |
 | Plugin disappears after Insight upgrade | Manifest field changed | Re-read official docs and update `package.json`. |
-| Large bundle, slow load | Missing minify, full lodash | Run `yarn minify:js`; switch to scoped imports. |
+| Large bundle, slow load | Missing minify, full lodash | Run `npm run minify:js`; switch to scoped imports. |

.ai/skills/redis-insight-plugin/references/external-parcel-plugin.md

@@ -41,7 +41,7 @@ Recommended scripts (see [../templates/external-parcel-package.json](../template
 ```json
 "scripts": {
   "start":   "parcel src/index.html",
-  "build":   "concurrently \"yarn build:js\" \"yarn build:css\"",
+  "build":   "concurrently \"npm run build:js\" \"npm run build:css\"",
   "build:js":"parcel build src/main.tsx --no-source-maps --dist-dir dist --target module",
   "build:css":"parcel build src/styles/styles.scss --no-source-maps --dist-dir dist",
   "minify:js":"terser dist/index.js -o dist/index.js -c -m",
@@ -95,7 +95,7 @@ Strip the rest before copying into `~/.redis-insight/plugins/<name>/`. The deplo
 
 ## Outputs
 
-After `yarn build`:
+After `npm run build`:
 
 - `dist/index.js` — single bundled module.
 - `dist/styles.css` — single stylesheet.
@@ -107,8 +107,8 @@ For RedisInsight product UI fidelity, copy `templates/external-styles.scss` to `
 ## Verification
 
 ```bash
-yarn build
-yarn verify     # runs templates/verify-plugin.sh
+npm run build
+npm run verify     # runs templates/verify-plugin.sh
 ```
 
 `verify` should report:

.ai/skills/redis-insight-plugin/references/iterative-development.md

@@ -145,8 +145,8 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-node@v4
         with: { node-version: '20' }
-      - run: yarn install --frozen-lockfile
-      - run: yarn build
+      - run: npm ci
+      - run: npm run build
       - run: bash scripts/verify-plugin.sh
       - uses: actions/upload-artifact@v4
         with:

.ai/skills/redis-insight-plugin/references/redis-insight-plugin-guidelines.md

@@ -78,7 +78,7 @@ export default { renderExampleView };
 
 ## Testing & Verification
 
-- `yarn build` produces `dist/index.js` and `dist/styles.css`.
+- `npm run build` produces `dist/index.js` and `dist/styles.css`.
 - `templates/verify-plugin.sh` confirms file presence, no `process.env`, and that activation method names appear in the bundle.
 - Deploy with `templates/deploy-external.sh` (folder install) or `templates/deploy-internal-docker.sh` (Docker container).
 - After deploy, `curl http://localhost:5540/api/plugins` and grep for the plugin name.

.ai/skills/redis-insight-plugin/references/testing-and-deployment.md

@@ -46,15 +46,15 @@ That's it. Nothing else is required at deploy time.
 External (host install):
 
 ```bash
-yarn build
+npm run build
 bash templates/verify-plugin.sh
 bash templates/deploy-external.sh
 ```
 
 Inside Docker RedisInsight:
 
 ```bash
-yarn build
+npm run build
 bash templates/verify-plugin.sh
 bash templates/deploy-internal-docker.sh
 ```