From d2f6ca0be80b49b47426dcec17fbe3bdb59addf3 Mon Sep 17 00:00:00 2001 From: Kai Haase Date: Sun, 12 Jul 2026 19:44:33 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(lt-dev):=20maintain=20aktualisiert=20Fr?= =?UTF-8?q?ameworks=20statt=20Security-Overrides=20zu=20l=C3=B6schen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ein realer Wartungslauf (offers, 2026-07) hat 20+ Security-Overrides gelöscht, den Build zerlegt und musste komplett zurückgerollt werden. Ursache waren zwei Anweisungen im Plugin selbst. 1) Zirkuläre Override-Regel Der Agent bekam vorgegeben: "If override was for security and pnpm audit shows no vulnerability -> REMOVE override". Das ist zirkulär — das Audit ist ja nur deshalb sauber, WEIL der Override greift. Die Regel garantierte damit, dass jeder wirksame Security-Override entfernt wird. Neu: Overrides werden ANGEHOBEN, nicht gelöscht. Entfernen verlangt positiven Beweis via `pnpm why`, dass das Paket den Baum verlassen hat. Der häufigste reale Befund ist ohnehin ein anderer: Pins, die exakt eine Patch-Version unter ihrem Fix liegen (im Realfall 8 von 36, u.a. vite 7.3.2 statt >=7.3.5) — exakt, gepflegt aussehend und trotzdem verwundbar. 2) Frameworks waren aus dem Scope ausgeschlossen nest-server und nuxt-extensions pinnen ihre Dependencies exakt. Eine CVE in einer solchen Dependency ist per Override NICHT schließbar — der Override überstimmt nur das Framework und erzeugt eine ungetestete Kombination. Genau das passierte mit better-auth: Audit grün, API-Test rot. Der richtige Fix war nest-server 11.25.2 -> 11.27.6, das die gepatchte Version selbst mitbringt. maintain ist jetzt ein Orchestrator: - Phase 0 erkennt die Topologie (Fullstack? npm- oder vendor-Mode je Projekt, anhand projects/api/src/core/ bzw. projects/app/app/core/). - Phase 1 aktualisiert die Frameworks und routet nach Modus: vendor an nest-server-core-updater / nuxt-extensions-core-updater, npm-Major an nest-server-updater (der die Migration Guides hat), npm-Minor direkt. - Phase 2 gleicht die Peers des Frameworks an (@nestjs/* usw.). Wird sonst vergessen und äußert sich als scheinbar unzusammenhängender Typfehler: nach dem nest-server-Update existierte @nestjs/schedule zweimal (gegen @nestjs/common 11.1.19 und 11.1.23) und der Build brach mit "Class 'CronJobs' incorrectly extends base class 'CoreCronJobs'". - Phase 3 übergibt an npm-package-maintainer, Phase 4 verifiziert (check + E2E). - Rollback-Protokoll: Snapshot vor Phase 1, bei nicht reparierbarem Bruch zurückrollen statt einen halb migrierten Dependency-Baum zu hinterlassen. Der npm-package-maintainer darf in Vendor-Projekten src/core/ bzw. app/core/ nicht anfassen — das ist Sache der core-updater. Angepasst: maintain, maintain-check, maintain-post-feature, maintain-security (dort die Ausnahme für framework-gepinnte Pakete), der Agent und der Skill. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../lt-dev/agents/npm-package-maintainer.md | 151 +++++++++++++----- .../commands/maintenance/maintain-check.md | 8 +- .../maintenance/maintain-post-feature.md | 7 +- .../commands/maintenance/maintain-security.md | 13 +- .../lt-dev/commands/maintenance/maintain.md | 148 +++++++++++++---- .../skills/maintaining-npm-packages/SKILL.md | 45 +++++- 6 files changed, 297 insertions(+), 75 deletions(-) diff --git a/plugins/lt-dev/agents/npm-package-maintainer.md b/plugins/lt-dev/agents/npm-package-maintainer.md index bd68526..9b7a9c0 100644 --- a/plugins/lt-dev/agents/npm-package-maintainer.md +++ b/plugins/lt-dev/agents/npm-package-maintainer.md @@ -24,9 +24,16 @@ This agent should be used when: ## Operation Modes **FULL MODE** (default): -- Execute all 4 priorities: Remove unused, optimize categorization, maximize updates, cleanup overrides +- Execute all priorities: remove unused, optimize categorization, maximize updates, + **raise** overrides to their fixed-in versions, align with the framework's pins - Complete optimization of the entire dependency ecosystem +**Vendor-mode boundary**: if the project carries the framework core in its own source +tree (`projects/api/src/core/`, `projects/app/app/core/`), those directories are +**off limits** for this agent. Updating them is the job of `nest-server-core-updater` +/ `nuxt-extensions-core-updater`, which the `/lt-dev:maintenance:maintain` command +runs before handing over to you. + **SECURITY-ONLY MODE**: - Skip Priority 1 & 2, focus ONLY on Priority 3 with security-critical updates - Update packages with known vulnerabilities, skip non-security updates @@ -70,11 +77,69 @@ This agent should be used when: - Balance update value against code change cost - **Goal**: Stay current for security and future-readiness -### Priority 4: CLEANUP OVERRIDES -- Review ALL existing `overrides` in package.json -- Remove overrides that are no longer necessary (parent package now includes fixed version) -- Keep only overrides that are still required for security or compatibility -- **Goal**: Minimize override complexity and maintenance burden +### Priority 4: MAINTAIN OVERRIDES — RAISE, DO NOT DELETE + +Existing overrides are **not clutter**. Nearly every entry is a security fix for a +transitive CVE that cannot be closed any other way, and the parallel `//overrides` +comment block records why. Treat that block as a contract. + +**The default action on an override is to RAISE it, never to remove it.** + +- Check every override target against its advisory's *fixed-in* version. An override + can be exact and still vulnerable — pinning `vite` to `7.3.2` when the fix landed + in `7.3.5` leaves the advisory wide open while looking maintained. Off-by-one pins + are the single most common finding; in a real audit 8 of 36 overrides were exactly + one patch below their fix. +- Raise to the highest release **within the same major** that is `>=` the fixed-in + version. Keep the target a fixed version — never `>=x`, `^x` or `~x`. +- After raising, re-run `audit` and confirm the package is gone from the report. + +**Removal requires positive proof, and only one of these two counts:** + +1. The direct dependency that pulled the vulnerable chain is gone from the project + (then the overrides that existed *only* for its chain go with it), or +2. `pnpm why ` shows the package is no longer in the tree at all. + +"The parent probably ships a fixed version now" is **not** proof. Verify with +`pnpm why` / `pnpm list `, and re-audit after every removal. + +**Never remove overrides to make a peer-dependency conflict go away.** If a build +breaks with a module-resolution error after a framework bump (e.g. +`Could not load .../useSiteConfig`), the cause is an outdated **meta-module**, not the +overrides. Raise the meta-module. + +Real incident (offers, 2026-07): a maintenance run deleted 20+ security overrides +(`axios`, `lodash`, `kysely`, `drizzle-orm`, `unhead`, `qs`, `hono`, …) to let the +Nuxt peer graph "re-resolve" itself. It reopened every one of those advisories, +broke the build anyway, and had to be reverted wholesale. The actual cause was +`@nuxtjs/seo@3.4.0` being too old for Nuxt 4.4 — one module bump, zero overrides +touched. + +### Priority 5: ALIGN WITH THE FRAMEWORK (fullstack projects) + +`@lenne.tech/nest-server` and `@lenne.tech/nuxt-extensions` pin their peers exactly. +When the project pins the same package to a *different* version, the package manager +installs **both** — and the build fails with type errors that point nowhere near the +real cause. + +After any framework version change, read the framework's own manifest and align: + +```bash +node -e "const p=require('@lenne.tech/nest-server/package.json'); + for (const [k,v] of Object.entries({...p.dependencies, ...p.peerDependencies})) + if (k.startsWith('@nestjs/')) console.log(k, v);" +``` + +Real incident (offers): after nest-server `11.25.2 → 11.27.6` the API build failed +with `Class 'CronJobs' incorrectly extends base class 'CoreCronJobs'`. Cause: +`@nestjs/schedule` resolved twice, once against the project's `@nestjs/common@11.1.19` +and once against the framework's `11.1.23`. Aligning `@nestjs/*` to `11.1.23` fixed it. + +**Never override a framework-pinned dependency to close a CVE.** That overrules the +framework and yields an untested combination — in the same incident, forcing +`better-auth` past nest-server's exact pin turned the audit green and an API test +red. Raise the *framework* instead; it ships the patched version. If no framework +release carries the fix yet, report it as blocked — do not force it. ### Constraints (Always Apply) @@ -473,43 +538,51 @@ Without this documentation, overrides become unmaintainable and accumulate indef 7. **Run validation:** ` install && run build && test` — all must pass. 8. **Verify the fix:** re-run `audit` and confirm the count drops as expected. If a package you just overrode STILL appears, the target is below the fixed-in version (step 2) or the selector missed the vulnerable instance — fix the override and re-audit. Do NOT record an override-able transitive vulnerability as "blocked" or "needs a framework update": that escalation is valid only after a correctly-targeted override has been proven not to clear it. -#### Removing Unnecessary Overrides +#### Auditing Existing Overrides (raise them — removal is the rare exception) -```bash -# 1. Check if overrides exist in package.json -grep -A 50 '"overrides"' package.json +⚠️ **A clean `pnpm audit` is NOT evidence that an override is obsolete.** The audit is +clean *because the override is doing its job*. Deleting it re-opens the advisory +immediately. Reasoning "no vulnerability reported → override no longer needed" is +circular, and it is exactly how a real maintenance run wiped 20+ security overrides. -# 2. For each override, check if it's still necessary: -``` +**For each override entry, in order:** -**For each override entry:** +1. **Read the `//overrides` comment** for that key. It states which CVE / transitive + chain the entry exists for, and — where applicable — the condition under which it + may be dropped. Honour it. -1. **Identify the override:** - ```json - "overrides": { - "package-name": "^1.2.3" - } +2. **Is the pin still above the fix?** Look up the advisory's fixed-in version and + compare against the current target: + ```bash + npm view versions --json # highest release within the SAME major ``` + Target below fixed-in → **RAISE** to the highest release in that major. + This is the common case. Eight of thirty-six overrides were one patch too low in + a single real audit. -2. **Check if parent packages now include the fixed version:** +3. **Is the package still in the tree at all?** ```bash - # See which packages depend on the overridden package - pnpm list package-name - - # Check what version would be installed without the override - pnpm view parent-package dependencies + pnpm why # nothing → the chain is gone ``` + Only a package that has genuinely left the dependency tree — usually because the + direct dependency that pulled it was removed — is a removal candidate. -3. **Decision logic:** - - If ALL parent packages now require the fixed version → **REMOVE override** - - If override was for security and `pnpm audit` shows no vulnerability → **REMOVE override** - - If still needed for compatibility or security → **KEEP override** +4. **Removal (rare):** delete the entry AND its `//overrides` comment, then + `pnpm install` and **re-audit**. If the advisory reappears, the removal was wrong: + restore the entry. -4. **Remove unnecessary overrides:** - - Edit package.json to remove the override entry - - Run `pnpm install` to update the lockfile - - Verify with `pnpm audit` that no new vulnerabilities appear - - Run `pnpm run build && pnpm test` to ensure compatibility +5. **Never remove an override to resolve a peer conflict or a build error.** That is + a symptom of an outdated module, not of a stale override. Raise the module. + +**Decision table** + +| Situation | Action | +| --- | --- | +| Target below the advisory's fixed-in version | **RAISE** to highest in same major | +| Target at/above fixed-in, package still in tree | **KEEP** unchanged | +| `pnpm why` shows the package is gone from the tree | **REMOVE** (entry + comment), then re-audit | +| Audit is clean | **KEEP** — that is the override working, not proof it is obsolete | +| Build breaks after a framework bump | **KEEP** — raise the outdated module instead | **Override Removal Checklist:** ```bash @@ -805,12 +878,16 @@ Before declaring success, verify ALL of these: 2. How many packages you moved to devDependencies (not left in dependencies) 3. How many deprecated packages you replaced with maintained alternatives 4. How many packages you updated with minimal/no code changes -5. How many unnecessary overrides you removed -5. Whether source code changes were kept to minimum +5. How many overrides you **raised** to their fixed-in version (and how many you + removed — with the `pnpm why` evidence for each removal) +6. Whether the project is aligned with its framework's pinned peers +7. Whether source code changes were kept to minimum **Your job priorities**: 1. Remove ALL unused packages first 2. Optimize categorization second 3. Update remaining packages third -4. Cleanup unnecessary overrides fourth -5. Only stop when all four goals are exhausted +4. Raise overrides to their fixed-in versions fourth — deleting a security override + without `pnpm why` proof is a regression, not a cleanup +5. Align `@nestjs/*` / `nuxt` / shared peers with the framework's own pins +6. Only stop when audit is 0, build passes in every project, and tests are green diff --git a/plugins/lt-dev/commands/maintenance/maintain-check.md b/plugins/lt-dev/commands/maintenance/maintain-check.md index 8e501f0..1f797c4 100644 --- a/plugins/lt-dev/commands/maintenance/maintain-check.md +++ b/plugins/lt-dev/commands/maintenance/maintain-check.md @@ -32,7 +32,8 @@ Analyze and report WITHOUT making changes: - Analyze categorization (what WOULD be moved to devDependencies) - Detect deprecated packages (what WOULD be replaced and with which alternatives) - Discover outdated packages (what WOULD be updated) -- Analyze overrides (which COULD be removed) +- Analyze overrides (which are pinned BELOW their advisory's fixed-in version and + should be raised; which — if any — have left the dependency tree per `pnpm why`) - Check security vulnerabilities - Estimate risk levels for all potential changes @@ -43,7 +44,10 @@ Generate comprehensive report including: - Packages that could be recategorized - Deprecated packages with recommended replacements - Available updates categorized by risk (SAFE/MEDIUM/HIGH) -- Overrides that may no longer be necessary (with analysis) +- Overrides pinned below their fixed-in version (with the target they should be raised to) +- Framework drift: is `@lenne.tech/nest-server` / `@lenne.tech/nuxt-extensions` behind, + and does any advisory sit in a dependency the framework pins? (Those cannot be fixed + with an override — they need the framework raised.) - Security vulnerabilities found - Estimated impact and time requirements diff --git a/plugins/lt-dev/commands/maintenance/maintain-post-feature.md b/plugins/lt-dev/commands/maintenance/maintain-post-feature.md index 1a2934f..2c66adb 100644 --- a/plugins/lt-dev/commands/maintenance/maintain-post-feature.md +++ b/plugins/lt-dev/commands/maintenance/maintain-post-feature.md @@ -33,7 +33,7 @@ Execute all priorities: 1. Remove unused packages (especially if feature changed dependencies) 2. Optimize dependency categorization (ensure new deps are correctly placed) 3. Update packages to latest versions (stay current after feature work) -4. Cleanup unnecessary overrides +4. **Raise** overrides that sit below their advisory's fixed-in version Check for: - Unused dependencies that can be removed (especially from feature work) @@ -42,7 +42,10 @@ Check for: - Outdated dependencies (all types: security, features, patches) - Security vulnerabilities introduced by new dependencies - Compatibility issues with newly added packages -- Overrides that are no longer necessary (parent packages now include fixed versions) +- Overrides pinned **below** their fixed-in version (raise them — this is the common + real finding). Overrides are never deleted just because the audit is clean; the + audit is clean *because* they work. Removal requires `pnpm why` proof that the + package has left the dependency tree. This is the standard comprehensive maintenance mode, ideal after completing feature development to ensure the codebase stays clean and dependencies stay current. diff --git a/plugins/lt-dev/commands/maintenance/maintain-security.md b/plugins/lt-dev/commands/maintenance/maintain-security.md index fd84129..c458dfd 100644 --- a/plugins/lt-dev/commands/maintenance/maintain-security.md +++ b/plugins/lt-dev/commands/maintenance/maintain-security.md @@ -41,4 +41,15 @@ Check for: This is a faster, minimal-change mode for urgent security fixes. -**Completion gate:** Re-run `audit` after changes and confirm the vulnerability count dropped to the expected residual. If a package that received an override still appears, the override target is too low or mis-scoped — fix it. Do NOT report a transitive advisory as "blocked" or "needs a framework update" before a correctly-targeted override has been proven unable to clear it. Ensure all tests and build pass after changes. +**Completion gate:** Re-run `audit` after changes and confirm the vulnerability count dropped to the expected residual. If a package that received an override still appears, the override target is too low or mis-scoped — fix it. Do NOT report a transitive advisory as "blocked" before a correctly-targeted override has been proven unable to clear it. Ensure all tests and build pass after changes. + +**Exception — framework-pinned dependencies.** If the vulnerable package is pinned by +`@lenne.tech/nest-server` or `@lenne.tech/nuxt-extensions` (check their +`package.json` → `dependencies`), an override is the WRONG tool: it overrules the +framework and yields an untested combination. Raise the **framework** instead — it +ships the patched version. If no framework release carries the fix yet, report it as +genuinely blocked. + +Real incident (offers, 2026-07): a critical `better-auth` advisory was overridden past +nest-server's exact pin. `pnpm audit` went green, an API test went red. The fix was +nest-server `11.25.2 → 11.27.6`, which pins the patched `better-auth` itself. diff --git a/plugins/lt-dev/commands/maintenance/maintain.md b/plugins/lt-dev/commands/maintenance/maintain.md index f3de65f..971d8b5 100644 --- a/plugins/lt-dev/commands/maintenance/maintain.md +++ b/plugins/lt-dev/commands/maintenance/maintain.md @@ -1,45 +1,129 @@ --- -description: Full npm package maintenance via npm-package-maintainer agent -allowed-tools: Agent +description: Full maintenance — framework updates (npm + vendor core) followed by npm package maintenance +allowed-tools: Agent, Bash, Read, Grep, Glob disable-model-invocation: true --- -# NPM Package Maintenance +# Full Project Maintenance -## When to Use This Command +Brings a project up to date **completely**: the lenne.tech frameworks first, then the +package ecosystem around them. Works for plain Node projects, fullstack monorepos +(`projects/api` + `projects/app`), and vendor-mode projects that carry the framework +core in their own source tree. -- Performing routine dependency maintenance on a Node.js project -- After completing multiple features and dependencies need cleanup -- When you want a comprehensive package optimization (removal, recategorization, updates) +## Why frameworks come first -## Related Commands +`@lenne.tech/nest-server` and `@lenne.tech/nuxt-extensions` **pin** their own +dependencies (e.g. nest-server pins `better-auth` to an exact version, and the +`@nestjs/*` family to an exact minor). Two consequences: -| Command | Mode | Use Case | -|---------|------|----------| -| `/lt-dev:maintenance:maintain` | FULL | Complete optimization (this command) | -| `/lt-dev:maintenance:maintain-check` | DRY-RUN | Analysis only - no changes | -| `/lt-dev:maintenance:maintain-security` | SECURITY | Fast security-only updates | -| `/lt-dev:maintenance:maintain-pre-release` | PRE-RELEASE | Conservative patch-only updates | -| `/lt-dev:maintenance:maintain-post-feature` | FULL | Clean up after feature development | +- A CVE inside a framework-pinned dependency **cannot be fixed with an override**. + Overriding it merely overrules the framework and produces a version combination + nobody tested. The fix is to raise the framework, which ships the patched version. +- Raising the framework moves its peer requirements. Any package maintenance done + *before* that is wasted work — it gets re-resolved anyway. -## User Prompt -Use the lt-dev:npm-package-maintainer agent to perform comprehensive npm package maintenance. +Real incident (offers, 2026-07): a critical `better-auth` advisory was "fixed" by +overriding `better-auth` past the version nest-server pinned. Audit went green, and +an API test went red. The actual fix was nest-server `11.25.2 → 11.27.6`, which +pins the patched `better-auth` itself. Order matters. -**Mode**: FULL MODE (complete optimization) +## Phase 0 — Detect topology (before spawning anything) -Execute all priorities: -1. Remove unused packages -2. Optimize dependency categorization -3. Update packages to latest versions -4. Cleanup unnecessary overrides +Never assume the layout. Determine, with Bash/Glob: -Check for: -- Unused dependencies that can be removed -- Packages that should be moved to devDependencies -- Deprecated packages and available replacements -- Outdated dependencies (all types: security, features, patches) -- Security vulnerabilities -- Compatibility issues -- Overrides that are no longer necessary (parent packages now include fixed versions) +**Which projects exist** +- Fullstack monorepo: `projects/api` + `projects/app` (also `packages/api` / `packages/app`) +- Single project: `package.json` at root -Ensure all tests and build pass after changes. +**Which mode each project is in** — this decides who does the framework update: + +| Check | Result | +| --- | --- | +| `projects/api/src/core/` exists | API is **vendor mode** (framework core lives in the repo) | +| `@lenne.tech/nest-server` in `projects/api/package.json` | API is **npm mode** | +| `projects/app/app/core/` exists | APP is **vendor mode** | +| `@lenne.tech/nuxt-extensions` in `projects/app/package.json` | APP is **npm mode** | + +A project can be vendor on one side and npm on the other. Report the detected +topology before doing anything. + +**Which framework versions are current** +```bash +npm view @lenne.tech/nest-server version +npm view @lenne.tech/nuxt-extensions version +``` + +## Phase 1 — Framework updates + +Route by mode. Do **not** hand vendor-mode work to the package maintainer — it must +not touch `src/core/` / `app/core/`. + +| Project | Mode | Version jump | Who | +| --- | --- | --- | --- | +| API | vendor | any | `lt-dev:nest-server-core-updater` agent | +| API | npm | **major** | `lt-dev:nest-server-updater` agent (owns the migration guides) | +| API | npm | minor / patch | raise in `package.json` directly, migration guides not needed | +| APP | vendor | any | `lt-dev:nuxt-extensions-core-updater` agent | +| APP | npm | any | raise in `package.json` directly; read the package CHANGELOG for breaking changes first | + +For npm-mode minor/patch bumps, read the framework CHANGELOG and check whether the +breaking changes listed actually touch this project (grep for the renamed symbols) +before raising. Record what you checked. + +## Phase 2 — Align the framework's ecosystem + +**This step is mandatory after every framework update and is routinely forgotten.** + +A framework pins peers. If the project pins them differently, you get *two* copies +of the same package and a build that fails with type errors that look unrelated. + +Real incident (offers): after nest-server `11.25.2 → 11.27.6`, the build failed with +`Class 'CronJobs' incorrectly extends base class 'CoreCronJobs'` — because +`@nestjs/schedule` existed twice, once resolved against `@nestjs/common@11.1.19` +(the project's pin) and once against `11.1.23` (the framework's). The fix is +mechanical: read the framework's own `package.json` and align. + +```bash +# what does the framework actually require? +node -e "const p=require('@lenne.tech/nest-server/package.json'); + for (const [k,v] of Object.entries({...p.dependencies, ...p.peerDependencies})) + if (k.startsWith('@nestjs/')) console.log(k, v);" +``` + +Align every shared package (`@nestjs/*`, `nuxt`, `vue`, …) to the framework's +version, then `pnpm install` and **build both projects** before moving on. + +## Phase 3 — Package maintenance + +Now hand off to the `lt-dev:npm-package-maintainer` agent (FULL mode). Pass it +explicitly: + +- the framework versions that are now in place (it must not fight them), +- that overrides are **raised, never deleted** (see the agent's Priority 4), +- that it must not touch `src/core/` or `app/core/` in vendor projects. + +## Phase 4 — Verify + +- `pnpm run check` (or the project's equivalent) must exit 0 — it typically bundles + audit, format, lint, tests, build and a server-start smoke test. +- If the project has E2E tests, run them. Framework updates change auth, SSR and + cookie behaviour; unit tests will not catch that. In lt projects: `lt dev test`. +- Report what changed, what is still open, and why. + +## Rollback + +Before Phase 1, snapshot every `package.json` and the lockfile. If any phase leaves +the build or tests broken and cannot be repaired within the phase, restore the +snapshot and report — **never** leave a half-migrated dependency tree behind. That +is worse than not having run at all: the lockfile no longer matches any tested state. + +## Related commands + +| Command | Scope | +| --- | --- | +| `/lt-dev:maintenance:maintain` | Frameworks + packages (this command) | +| `/lt-dev:maintenance:maintain-check` | Analysis only, no changes | +| `/lt-dev:maintenance:maintain-security` | Security-only, fast path | +| `/lt-dev:maintenance:maintain-pre-release` | Conservative, patch-only | +| `/lt-dev:maintenance:maintain-post-feature` | Cleanup after feature work | diff --git a/plugins/lt-dev/skills/maintaining-npm-packages/SKILL.md b/plugins/lt-dev/skills/maintaining-npm-packages/SKILL.md index f2144b8..0fa2b1a 100644 --- a/plugins/lt-dev/skills/maintaining-npm-packages/SKILL.md +++ b/plugins/lt-dev/skills/maintaining-npm-packages/SKILL.md @@ -36,9 +36,28 @@ For comprehensive npm package maintenance, use the **lt-dev:npm-package-maintain | "Update npm packages" | **THIS SKILL** | | "npm audit fix" | **THIS SKILL** | | "Remove unused dependencies" | **THIS SKILL** | -| "Update nest-server to v14" | nest-server-updating | +| "Bring the whole project up to date" | `/lt-dev:maintenance:maintain` — orchestrates framework updates first, then this skill | +| "Update nest-server across a major" | nest-server-updating (owns the migration guides) | +| "Sync the vendored core (`src/core/`)" | nest-server-core-vendoring / nuxt-extensions-core-vendoring | | "Fix NestJS service" | generating-nest-servers | +### Frameworks come before packages + +A CVE inside a **framework-pinned** dependency cannot be closed with an override. +`@lenne.tech/nest-server` pins `better-auth` and the `@nestjs/*` family to exact +versions; overriding past that pin overrules the framework and produces a combination +nobody tested. Raise the framework instead — it ships the patched version. + +Real incident (offers, 2026-07): a critical `better-auth` advisory was "fixed" via +override. `pnpm audit` went green and an API test went red. The correct fix was +nest-server `11.25.2 → 11.27.6`. Afterwards the `@nestjs/*` family had to be aligned +to the framework's pin (`11.1.23`), because two copies of `@nestjs/schedule` broke the +build with a type error that pointed nowhere near the cause. + +Practical consequence: when a security finding sits in a framework-pinned dependency, +**stop and raise the framework** (or report it blocked if no release carries the fix). +Do not force it with an override. + ## Related Skills - `generating-nest-servers` - For NestJS development when dependencies affect the server @@ -118,6 +137,30 @@ When the agent ADDS an entry to `pnpm.overrides` (typically to force a security- **Reference implementation:** `https://github.com/lenneTech/nest-server-starter/blob/main/package.json` — canonical example of correctly-written `pnpm.overrides` for the lenne.tech stack. Align with this file when in doubt. The detailed rule is in `@lenne.tech/nest-server/.claude/rules/package-management.md` → "Overrides". +## Override Retention Rule (Critical) + +**Raise overrides. Do not delete them.** Every entry in `pnpm.overrides` is there +because a transitive CVE could not be closed any other way, and the parallel +`//overrides` block records which. Treat it as a contract. + +⚠️ **A clean `pnpm audit` is not evidence that an override is obsolete** — the audit is +clean *because the override is working*. "No vulnerability reported, so the override +can go" is circular reasoning, and it is precisely how a real maintenance run deleted +20+ security overrides (`axios`, `lodash`, `kysely`, `drizzle-orm`, `unhead`, `qs`, +`hono`, …), re-opening every one of those advisories. + +| Situation | Action | +| --- | --- | +| Target below the advisory's fixed-in version | **RAISE** to the highest release in the same major | +| Target at/above fixed-in, package still in the tree | **KEEP** | +| `pnpm why ` shows the package left the tree | **REMOVE** (entry + its `//overrides` comment), then re-audit | +| Audit is clean | **KEEP** | +| Build breaks after a framework bump | **KEEP** — raise the outdated meta-module instead | + +Off-by-one pins are the most common real finding: in one audit, 8 of 36 overrides sat +exactly one patch below their fix (e.g. `vite` pinned to `7.3.2` when the advisory was +fixed in `7.3.5`) — exact, maintained-looking, and still vulnerable. + ## Vulnerability Resolution Workflow When `audit` reports vulnerabilities, resolve them in this order. Most are fixable without a major upgrade or a framework bump — escalation is the last resort, not the first diagnosis. From 24c188b4bc8a5c9d91dd4d479db9b7238d751100 Mon Sep 17 00:00:00 2001 From: Kai Haase Date: Mon, 13 Jul 2026 19:54:19 +0200 Subject: [PATCH 2/3] chore: bump version to 7.57.0 enforce draft-only PRs and forbid merges in core contributors --- package-lock.json | 4 +-- package.json | 2 +- plugins/lt-dev/.claude-plugin/plugin.json | 2 +- .../agents/nest-server-core-contributor.md | 25 +++++++++++++---- .../nuxt-extensions-core-contributor.md | 28 +++++++++++++++---- .../backend/contribute-nest-server-core.md | 12 ++++++++ .../contribute-nuxt-extensions-core.md | 12 ++++++++ plugins/lt-offers/.claude-plugin/plugin.json | 2 +- .../lt-showroom/.claude-plugin/plugin.json | 2 +- 9 files changed, 72 insertions(+), 17 deletions(-) diff --git a/package-lock.json b/package-lock.json index 647f7be..281979d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@lenne.tech/claude-code", - "version": "7.56.0", + "version": "7.57.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@lenne.tech/claude-code", - "version": "7.56.0", + "version": "7.57.0", "license": "MIT", "devDependencies": { "@types/bun": "1.3.4", diff --git a/package.json b/package.json index 0f0d9fb..bb48234 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lenne.tech/claude-code", - "version": "7.56.0", + "version": "7.57.0", "description": "lenne.tech Development Skills, Commands and Hooks for Claude Code", "private": true, "scripts": { diff --git a/plugins/lt-dev/.claude-plugin/plugin.json b/plugins/lt-dev/.claude-plugin/plugin.json index 214e1d4..2a7639b 100644 --- a/plugins/lt-dev/.claude-plugin/plugin.json +++ b/plugins/lt-dev/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "lt-dev", - "version": "7.56.0", + "version": "7.57.0", "description": "lenne.tech Development Skills, Commands and Hooks for Claude Code - Frontend (Nuxt 4), Backend (NestJS), TDD and CLI Tools", "author": { "name": "lenne.tech GmbH", diff --git a/plugins/lt-dev/agents/nest-server-core-contributor.md b/plugins/lt-dev/agents/nest-server-core-contributor.md index 0cff8d1..6127774 100644 --- a/plugins/lt-dev/agents/nest-server-core-contributor.md +++ b/plugins/lt-dev/agents/nest-server-core-contributor.md @@ -41,14 +41,29 @@ Use this agent when: ## Operating Principles -1. **Never auto-push.** Every PR goes through human review before submission. -2. **Cosmetic filter is strict.** Formatting, linting, whitespace, quote-style +1. **The mandate ends at an open PR — never merge.** The furthest you may ever go + is: feature branch → **open PR into `dev`/`develop`** (or into `main` only when + the repo has no `dev`/`develop` branch). Not one step further. You never merge + the PR, never advance `dev`/`develop` → `main`, never tag, never bump a version + beyond what the repo's PR convention requires, never publish to npm. Those are + the maintainer's decisions, every time, in every repo. This holds even when an + earlier instruction sounded like blanket approval ("merge everything", "bring + it into dev and then main") — an upstream merge is always a separate, explicit + decision the user makes, not one you infer. +2. **Never auto-push.** Every PR goes through human review before submission. + If — and only if — the user explicitly asks you to push the branch or open the + PR, doing so is the END of your mandate, not the start of a merge. +3. **Any PR you open is a DRAFT.** Always `gh pr create --draft`. A ready PR can be + merged by anyone who walks past it — a draft cannot. Never mark a PR + ready-for-review yourself (`gh pr ready`); flipping it out of draft is the + maintainer's signal that they have decided, and that signal is theirs to give. +4. **Cosmetic filter is strict.** Formatting, linting, whitespace, quote-style changes are filtered out even if commit message doesn't mark them. -3. **Project-specific is respected.** Customer-name references, business-rule +5. **Project-specific is respected.** Customer-name references, business-rule enums, proprietary integration code stays local. -4. **Generate PR drafts, not direct PRs.** The agent prepares a branch in a +6. **Generate PR drafts, not direct PRs.** The agent prepares a branch in a local clone of upstream — human inspects, pushes, opens PR via normal GitHub. -5. **Cross-reference upstream.** If a similar change already exists upstream +7. **Cross-reference upstream.** If a similar change already exists upstream (either in the current version or in an open PR), link it instead of creating a duplicate. diff --git a/plugins/lt-dev/agents/nuxt-extensions-core-contributor.md b/plugins/lt-dev/agents/nuxt-extensions-core-contributor.md index 12ada16..f4001c2 100644 --- a/plugins/lt-dev/agents/nuxt-extensions-core-contributor.md +++ b/plugins/lt-dev/agents/nuxt-extensions-core-contributor.md @@ -41,17 +41,33 @@ Use this agent when: ## Operating Principles -1. **Never auto-push.** Every PR goes through human review before submission. -2. **Cosmetic filter is strict.** Formatting, linting, whitespace, quote-style +1. **The mandate ends at an open PR -- never merge.** The furthest you may ever go + is: feature branch -> **open PR into `dev`/`develop`** (or into `main` only when + the repo has no `dev`/`develop` branch -- which is the case for nuxt-extensions + today). Not one step further. You never merge the PR, never advance + `dev`/`develop` -> `main`, never tag, never bump a version beyond what the + repo's PR convention requires, never publish to npm. Those are the maintainer's + decisions, every time, in every repo. This holds even when an earlier + instruction sounded like blanket approval ("merge everything", "bring it into + dev and then main") -- an upstream merge is always a separate, explicit + decision the user makes, not one you infer. +2. **Never auto-push.** Every PR goes through human review before submission. + If -- and only if -- the user explicitly asks you to push the branch or open + the PR, doing so is the END of your mandate, not the start of a merge. +3. **Any PR you open is a DRAFT.** Always `gh pr create --draft`. A ready PR can be + merged by anyone who walks past it -- a draft cannot. Never mark a PR + ready-for-review yourself (`gh pr ready`); flipping it out of draft is the + maintainer's signal that they have decided, and that signal is theirs to give. +4. **Cosmetic filter is strict.** Formatting, linting, whitespace, quote-style changes are filtered out even if commit message doesn't mark them. -3. **Project-specific is respected.** Customer-name references, business-rule +5. **Project-specific is respected.** Customer-name references, business-rule enums, proprietary integration code stays local. -4. **Generate PR drafts, not direct PRs.** The agent prepares a branch in a +6. **Generate PR drafts, not direct PRs.** The agent prepares a branch in a local clone of upstream -- human inspects, pushes, opens PR via normal GitHub. -5. **Cross-reference upstream.** If a similar change already exists upstream +7. **Cross-reference upstream.** If a similar change already exists upstream (either in the current version or in an open PR), link it instead of creating a duplicate. -6. **No reverse flatten-fix needed.** Unlike backend contributions where import +8. **No reverse flatten-fix needed.** Unlike backend contributions where import paths must be un-flattened, nuxt-extensions uses 1:1 path mapping between the vendored tree and upstream. diff --git a/plugins/lt-dev/commands/backend/contribute-nest-server-core.md b/plugins/lt-dev/commands/backend/contribute-nest-server-core.md index 5841181..a65a8c6 100644 --- a/plugins/lt-dev/commands/backend/contribute-nest-server-core.md +++ b/plugins/lt-dev/commands/backend/contribute-nest-server-core.md @@ -133,6 +133,18 @@ Execute the contributor workflow: NEVER auto-push. NEVER open PRs automatically. Every PR draft is for human review and manual submission. +THE CEILING IS AN OPEN DRAFT PR — NEVER A MERGE. The furthest this flow may ever +go is: feature branch → open **draft** PR into `dev`/`develop` (or into `main` only +when the repo has no `dev`/`develop` branch). Not one step further. Never merge the +PR, never advance `dev`/`develop` → `main`, never tag, never release, never publish +to npm. Those are always the maintainer's decision — including when an earlier +instruction sounded like blanket approval ("merge everything", "bring it into dev +and then main"). Ask; do not infer. + +ALWAYS `gh pr create --draft`. A ready PR can be merged by anyone who walks past +it; a draft cannot. Never run `gh pr ready` — taking a PR out of draft is the +maintainer's way of saying they have decided, and that signal is theirs to give. + Project-specific signals to reject: Volksbank, imo, customer-specific enums, business-rule hardcoded values, API endpoints with customer domains. diff --git a/plugins/lt-dev/commands/frontend/contribute-nuxt-extensions-core.md b/plugins/lt-dev/commands/frontend/contribute-nuxt-extensions-core.md index 5a16abd..3f92476 100644 --- a/plugins/lt-dev/commands/frontend/contribute-nuxt-extensions-core.md +++ b/plugins/lt-dev/commands/frontend/contribute-nuxt-extensions-core.md @@ -135,5 +135,17 @@ Execute the contributor workflow: NEVER auto-push. NEVER open PRs automatically. Every PR draft is for human review and manual submission. +THE CEILING IS AN OPEN DRAFT PR — NEVER A MERGE. The furthest this flow may ever +go is: feature branch → open **draft** PR into `dev`/`develop` (or into `main` only +when the repo has no `dev`/`develop` branch — which is the case for nuxt-extensions +today). Not one step further. Never merge the PR, never advance `dev`/`develop` → +`main`, never tag, never release, never publish to npm. Those are always the +maintainer's decision — including when an earlier instruction sounded like blanket +approval ("merge everything", "bring it into dev and then main"). Ask; do not infer. + +ALWAYS `gh pr create --draft`. A ready PR can be merged by anyone who walks past +it; a draft cannot. Never run `gh pr ready` — taking a PR out of draft is the +maintainer's way of saying they have decided, and that signal is theirs to give. + Work fully autonomously otherwise. ``` diff --git a/plugins/lt-offers/.claude-plugin/plugin.json b/plugins/lt-offers/.claude-plugin/plugin.json index ec4cb98..f03012c 100644 --- a/plugins/lt-offers/.claude-plugin/plugin.json +++ b/plugins/lt-offers/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "lt-offers", - "version": "7.56.0", + "version": "7.57.0", "description": "Skills & Commands for creating and managing business offers on angebote.lenne.tech (production) and demo-angebote.lenne.tech (demo) — MCP tools for offers, knowledge base, analytics, and content management", "author": { "name": "lenne.tech GmbH", diff --git a/plugins/lt-showroom/.claude-plugin/plugin.json b/plugins/lt-showroom/.claude-plugin/plugin.json index e9d95df..ca63413 100644 --- a/plugins/lt-showroom/.claude-plugin/plugin.json +++ b/plugins/lt-showroom/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "lt-showroom", - "version": "7.56.0", + "version": "7.57.0", "description": "Skills & Commands for analyzing software projects and creating showcases on showroom.lenne.tech — MCP tools for showcases, screenshots, analytics, and content management", "author": { "name": "lenne.tech GmbH", From e6a7e6454b150d889bb7eb8692645e65d8292412 Mon Sep 17 00:00:00 2001 From: Kai Haase Date: Fri, 17 Jul 2026 06:04:15 +0200 Subject: [PATCH 3/3] chore: bump version to 7.58.0 add e2e DB isolation reference and harden update/check workflows --- package-lock.json | 4 +- package.json | 2 +- plugins/lt-dev/.claude-plugin/plugin.json | 2 +- .../lt-dev/commands/fullstack/update-all.md | 48 ++++-- .../contributing-to-lt-framework/SKILL.md | 29 +++- .../skills/generating-nest-servers/SKILL.md | 23 +++ .../reference/e2e-database-isolation.md | 150 ++++++++++++++++++ .../nest-server-core-vendoring/SKILL.md | 39 +++++ .../nuxt-extensions-core-vendoring/SKILL.md | 30 ++++ .../skills/running-check-script/SKILL.md | 24 +++ plugins/lt-offers/.claude-plugin/plugin.json | 2 +- .../lt-showroom/.claude-plugin/plugin.json | 2 +- 12 files changed, 333 insertions(+), 22 deletions(-) create mode 100644 plugins/lt-dev/skills/generating-nest-servers/reference/e2e-database-isolation.md diff --git a/package-lock.json b/package-lock.json index 281979d..c550957 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@lenne.tech/claude-code", - "version": "7.57.0", + "version": "7.58.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@lenne.tech/claude-code", - "version": "7.57.0", + "version": "7.58.0", "license": "MIT", "devDependencies": { "@types/bun": "1.3.4", diff --git a/package.json b/package.json index bb48234..3090458 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lenne.tech/claude-code", - "version": "7.57.0", + "version": "7.58.0", "description": "lenne.tech Development Skills, Commands and Hooks for Claude Code", "private": true, "scripts": { diff --git a/plugins/lt-dev/.claude-plugin/plugin.json b/plugins/lt-dev/.claude-plugin/plugin.json index 2a7639b..9d3a5f2 100644 --- a/plugins/lt-dev/.claude-plugin/plugin.json +++ b/plugins/lt-dev/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "lt-dev", - "version": "7.57.0", + "version": "7.58.0", "description": "lenne.tech Development Skills, Commands and Hooks for Claude Code - Frontend (Nuxt 4), Backend (NestJS), TDD and CLI Tools", "author": { "name": "lenne.tech GmbH", diff --git a/plugins/lt-dev/commands/fullstack/update-all.md b/plugins/lt-dev/commands/fullstack/update-all.md index b7e9d7a..4b9400a 100644 --- a/plugins/lt-dev/commands/fullstack/update-all.md +++ b/plugins/lt-dev/commands/fullstack/update-all.md @@ -87,6 +87,22 @@ Parse `$ARGUMENTS` for flags: - `--skip-frontend`: Skip frontend (App) update - `--skip-packages`: Skip package maintenance phase +### Phase 0: CLI Self-Heals (unless --dry-run) + +Run the lt CLI's deterministic self-heals FIRST — they are NOT covered by any +agent phase below and take ~2 seconds: + +```bash +lt fullstack update +``` + +This idempotently (a) syncs `scripts/check.mjs` to the CLI's canonical version +(idle-watchdog, hoisted install, summed test metrics — skips the sync when the +file has UNCOMMITTED local edits and says so), (b) adds `.lt-dev/` to +`.gitignore` if missing, and (c) refreshes the CLAUDE.md vendor-notice blocks. +It also prints the mode detection, which cross-checks Phase 1. Report anything +it changed (or skipped) in the final report's summary. + ### Phase 1: Detect Modes 1. **Detect project structure:** @@ -286,20 +302,21 @@ Only sync the targets that were actually updated. Use section-level merge ### Phase 7: Cross-Validation -1. **Build both projects:** +1. **Run the canonical workspace check** (from the workspace root): ```bash - cd && pnpm run build - cd && pnpm run build + pnpm run check ``` + This supersedes separate per-project build/test runs: the report-driven + wrapper executes audit + format + lint + unit/API tests + build + + server-start for BOTH subprojects, with the idle-watchdog guarding the test + steps and the e2e-run governor queuing against parallel sessions. A run + printing `[e2e-governor] waiting for a free e2e slot` every 15s is QUEUED + behind another session's tests, not hung — let it wait. Must exit 0; apply + the `running-check-script` skill's iterate-until-green loop on failures. -2. **Run tests:** - ```bash - cd && pnpm test - ``` - -3. **Confirm the security audit** (use the project's own package manager) in each subproject — the residual vulnerability count must match what Phase 5 reported as expected. A package that received an override but still appears means the override target is too low or mis-scoped; do not sign off the update until it is fixed or explicitly accepted with a documented reason. +2. **Confirm the security audit** (use the project's own package manager) in each subproject — the residual vulnerability count must match what Phase 5 reported as expected. A package that received an override but still appears means the override target is too low or mis-scoped; do not sign off the update until it is fixed or explicitly accepted with a documented reason. -4. **Verify type generation works — conditional:** +3. **Verify type generation works — conditional:** ```bash cd if grep -REq "from ['\"](~|\.|app)/api-client" app/; then @@ -313,6 +330,17 @@ Only sync the targets that were actually updated. Use section-level merge reference artifact, and the prettier/oxfmt format difference surfaces as a false `check` failure the operator has to chase. +4. **Run the environment diagnostics** (from the workspace root): + ```bash + lt dev doctor + ``` + Post-update it verifies exactly what the update was supposed to fix: + `scripts/check.mjs` matches the canonical CLI version (drift warning), + the Playwright global-setup allow-list is ticket/shard-safe, and the + slug/registry/Caddy state is consistent. Treat every WARN as a report + item — fix what the update caused; pre-existing environment WARNs + (e.g. Caddy service issues) are reported but do not block sign-off. + ### Phase 8: Report Generate a comprehensive report: diff --git a/plugins/lt-dev/skills/contributing-to-lt-framework/SKILL.md b/plugins/lt-dev/skills/contributing-to-lt-framework/SKILL.md index 1dc41cc..eb6f323 100644 --- a/plugins/lt-dev/skills/contributing-to-lt-framework/SKILL.md +++ b/plugins/lt-dev/skills/contributing-to-lt-framework/SKILL.md @@ -26,18 +26,35 @@ Framework-level development happens in the **framework repositories**, not in co | "Sync vendored core from upstream" | nest-server-core-vendoring / nuxt-extensions-core-vendoring | | "Open a PR with my vendored-core change" | nest-server-core-vendoring → `nest-server-core-contributor` agent | -## Prerequisites +## The Base Repos ("Grund-Repos") -Four git repositories must be available locally on the user's machine, all cloned from `github.com/lenneTech/`: +When the user says **"Grund-Repos"** (or "base repos" / "the foundation repos"), they mean **exactly these seven** — the repositories every customer project inherits from: | Repo | Role | |------|------| | `nest-server` | Backend framework source | -| `nest-server-starter` | Backend template that consumes `@lenne.tech/nest-server` | -| `nuxt-extensions` | Frontend framework source | -| `nuxt-base-starter` | Frontend template that consumes `@lenne.tech/nuxt-extensions` | +| `nest-server-starter` | Backend template → `projects/api` | +| `nuxt-extensions` | Frontend library source | +| `nuxt-base-starter` | Frontend template → `projects/app` | +| `lt-monorepo` | Fullstack monorepo template | +| `cli` | The `lt` command line (`lt dev`, `lt ticket`, `lt fullstack`, …) | +| `lt-dev` | This Claude Code plugin (`claude-code/plugins/lt-dev`) — commands, agents, skills | + +All are cloned from `github.com/lenneTech/` and live side by side in one directory on the user's machine (ask for the path, or detect it — do not hardcode). `lt-dev` is a subdirectory of the `claude-code` marketplace repo, not a repo of its own. + +### The rule that makes them matter + +**A defect that every project in the stack would inherit belongs in the base repo — not (only) in the customer project.** A local patch fixes one project; the base repo fixes every project that will ever be created. + +Two consequences, both easy to get wrong: + +1. **Look upstream BEFORE building your own.** When a project-level problem looks structural (test setup, dev-server orchestration, config layout, auth wiring), first check whether the base repo already solved it. Reinventing it locally creates a divergence that breaks on the next framework update — and the base repo's version is usually the better-tested one. + *Real case:* a customer project's API test suite kept failing at random under parallel runs because all working copies shared one e2e database that the global setup drops on start. `nest-server` and `nest-server-starter` had solved this long before (a database per test RUN, plus a lifecycle reporter that cleans up). The project had simply never adopted it. The fix was to port the framework solution — not to invent a third scheme. +2. **Push project-grown improvements back up.** A guard, fix, or hardening that was written in a project because the framework lacked it is a **contribution owed upstream** (see the workflows below). Same case: the project had a safety guard refusing to drop a database that is not recognizably a test DB — the base repos did **not** have it, and without it a running `lt dev` session (which points `MONGODB_URI` / `NSC__MONGOOSE__URI` at the DEVELOPMENT database) would let a test run wipe the developer's data. + +## Prerequisites -The exact filesystem location is user-specific (e.g. side-by-side sibling directories). Ask the user for the paths at the start of the workflow, or detect them via shell history / common parent directories. All commands below use **`$FRAMEWORK_DIR`** and **`$STARTER_DIR`** as placeholders — resolve them to the user's actual paths before execution. +For the link workflows below, resolve **`$FRAMEWORK_DIR`** and **`$STARTER_DIR`** to the user's actual clone paths before executing any command. ## Workflow A — Backend Framework (`@lenne.tech/nest-server`) diff --git a/plugins/lt-dev/skills/generating-nest-servers/SKILL.md b/plugins/lt-dev/skills/generating-nest-servers/SKILL.md index 3e9f9b8..6e390d6 100644 --- a/plugins/lt-dev/skills/generating-nest-servers/SKILL.md +++ b/plugins/lt-dev/skills/generating-nest-servers/SKILL.md @@ -269,6 +269,29 @@ afterAll(async () => { **Use separate test database:** `app-test` instead of `app-dev` +### E2E Database Isolation — the shared-DB 401 flake (CRITICAL) + +E2e specs run in **parallel forks** sharing **one** database. A spec that clears a GLOBAL collection +(BetterAuth's `jwks` signing keys, `users`, `session`, `ratelimitstates`, …) corrupts every parallel +file mid-run → **spurious 401** / missing data. It passes in isolation and only fails under the full +run, so it is routinely misdiagnosed as "just flaky — re-run it". **It is a real concurrency bug.** + +Fix = **per-worker DB isolation**: `tests/setup.ts` (a `setupFiles` entry, runs per fork before +`config.env.ts`) appends the vitest pool id to the DB URI so each concurrent fork gets its own DB: + +```typescript +if (process.env.MONGODB_URI && !/-w\d+(\?|$)/.test(process.env.MONGODB_URI)) { + const poolId = process.env.VITEST_POOL_ID || process.env.VITEST_WORKER_ID || '0'; + process.env.MONGODB_URI = process.env.MONGODB_URI.replace(/(\/[^/?]+)(\?|$)/, `$1-w${poolId}$2`); +} +``` + +**And** the `db-lifecycle.reporter.ts` cleanup of other runs' DBs MUST be `isPidAlive` + age guarded — +an unconditional pattern-drop wipes a *concurrent* run's live databases. + +**Full mechanism, cross-run-safety proof, anti-patterns, and a review checklist: +[reference/e2e-database-isolation.md](${CLAUDE_SKILL_DIR}/reference/e2e-database-isolation.md)** + ## Framework Source Files (MUST READ before guessing) **ALWAYS read actual source code** before guessing framework behavior. lenne.tech projects ship the framework source in one of two consumption modes: diff --git a/plugins/lt-dev/skills/generating-nest-servers/reference/e2e-database-isolation.md b/plugins/lt-dev/skills/generating-nest-servers/reference/e2e-database-isolation.md new file mode 100644 index 0000000..369dbf1 --- /dev/null +++ b/plugins/lt-dev/skills/generating-nest-servers/reference/e2e-database-isolation.md @@ -0,0 +1,150 @@ +# E2E Test Database Isolation (the shared-DB 401 flake) + +A recurring, hard-to-diagnose flake in lenne.tech nest-server projects: e2e tests that pass in +isolation fail intermittently under the full parallel run — most visibly as a **spurious 401** +(`expected 200 to be 401` on an authenticated request), or missing/altered data. It reads like +flakiness; it is a **real concurrency bug** in the test-database lifecycle. This doc explains the +mechanism, the fix, and — critically — why the cleanup must be pid-guarded. + +## The mechanism + +The e2e config runs spec **files in parallel forks** (`fileParallelism: true`, `pool: 'forks'`). +`tests/global-setup.ts` gives the whole **run** one database and exports it via `MONGODB_URI` +(or `NSC__MONGOOSE__URI` in NSC-config projects) *before the forks spawn*. So every parallel file +shares **one** database. + +That is fine until a spec **mutates a GLOBAL collection** — one that other files depend on +implicitly. The canonical trigger is BetterAuth's `jwks` collection (the JWT signing keys, used by +*every* BetterAuth instance): + +```typescript +// better-auth-integration.story.test.ts — runs in a parallel fork +beforeAll(async () => { + await db.collection('jwks').deleteMany({}); // clears the signing keys for the WHOLE shared DB +}); +``` + +While that runs, a parallel `better-auth-*` file has a user signed in with a token signed by those +keys. The clear wipes the keys → that file's next authenticated request fails verification → **401**. +Same class of bug for any `deleteMany({})` / `dropDatabase()` / `.drop()` on a collection another +parallel file reads: `users`, `session`, framework collections like `ratelimitstates`, etc. + +**It only manifests under parallel load**, so it is routinely misfiled as "flaky test — just re-run". +It is not. Isolated, the file passes; in parallel, it collides. + +## The fix — per-worker database isolation + +Give each **concurrent fork** its own database, so parallel files can never share collections. The +hook is `tests/setup.ts` (a `setupFiles` entry): it runs **in every fork, before the test file and +therefore before `config.env.ts` is imported**, so it can rewrite the DB URI: + +```typescript +// tests/setup.ts (registered via `setupFiles: ['tests/setup.ts']` in vitest-e2e.config.ts) +if (process.env.MONGODB_URI && !/-w\d+(\?|$)/.test(process.env.MONGODB_URI)) { + const poolId = process.env.VITEST_POOL_ID || process.env.VITEST_WORKER_ID || '0'; + // …/dbname?opts → …/dbname-wN?opts + process.env.MONGODB_URI = process.env.MONGODB_URI.replace(/(\/[^/?]+)(\?|$)/, `$1-w${poolId}$2`); +} +``` + +Result: fork 1 uses `-run--p-w1`, fork 2 `-w2`, etc. Files that share a fork run +**sequentially** (safe); files in **different** forks can no longer collide. Use the exact env var +your `global-setup.ts` sets — `MONGODB_URI` (framework) or `NSC__MONGOOSE__URI` (NSC-config +projects). It must run before the config is imported, so keep `setup.ts` free of imports that pull +in `config.env.ts`. + +> A project may instead derive the DB name in `config.env.ts` from `VITEST_POOL_ID` directly — same +> effect. What matters is that concurrent forks get distinct DBs. + +## The cleanup MUST be pid-guarded (or you delete a live run's data) + +`tests/db-lifecycle.reporter.ts` runs at the **end** of a run and drops that run's databases. The +trap: it also collects *other* runs' leftover DBs. It may drop a foreign DB **only** if its creating +process is dead or it is very old: + +```typescript +} else if (name.startsWith(`${base}-run-`)) { + const match = name.match(/-run-(\d+)-p(\d+)/); + stale = match + ? !isPidAlive(Number(match[2])) || Date.now() - Number(match[1]) > STALE_MAX_AGE_MS // 7 days + : false; +} +// …and it drops THIS run's own per-worker forks via `name.startsWith(`${dbName}-`)`. +``` + +This is what makes concurrent runs safe: two runs (two `lt ticket` worktrees, CI + local) get +distinct pids → distinct DB names, and the `isPidAlive` guard means a finishing run **cannot** drop a +DB whose owning run is still alive. Verified empirically: run A's cleanup executing while run B is +live left B's 137 tests untouched, zero orphaned DBs afterwards. + +**Anti-pattern to reject on sight** — an unguarded pattern-drop in `global-setup.ts`: + +```typescript +// UNSAFE: wipes every sibling DB with no pid/age guard. A concurrent run's LIVE fork DBs get dropped. +for (const { name } of databases) { + if (name.startsWith('platform-e2e')) await connection.db(name).dropDatabase(); +} +``` + +If per-fork DBs are **fixed-named** (`platform-e2e-`, reused every run) instead of +per-run-unique (`…-run--p-w`), an unguarded drop in one run's *startup* wipes +another run's *live* worker DBs. Always: per-run-unique names **and** an `isPidAlive`/age-guarded +reporter. + +## Startup sweep — cleanup that survives SIGKILL and `--reporter` overrides + +The end-of-run reporter can NEVER run when the process is SIGKILLed (check-script watchdog +escalation, closed terminal) or when vitest was started with an explicit `--reporter` CLI flag +(which **replaces** the config reporters). Relying on "the next successful run collects leftovers" +means leaks persist exactly when runs keep failing. The fix: `global-setup.ts` sweeps **before** the +run starts — same `isPidAlive`/age predicate (shared `isStaleTestDb()` export in the reporter), same +`SAFE_TEST_DB_PATTERN` guard. Restarting a suite therefore always restores a clean state, no matter +how its predecessor died. Reference: `tests/global-setup.ts` + `tests/db-lifecycle.reporter.ts` in +nest-server/nest-server-starter (11.29.0+). + +## Machine-wide e2e run governor (`tests/e2e-run-slots.ts`) + +vitest's default fork count (`numCpus - 1`) assumes an EXCLUSIVE machine. Two overlapping full-speed +e2e runs (two `lt ticket` sessions running `check`) saturate the box — measured: load 30 on 12 +cores, spurious 401s, hard failures. The governor is a cross-process slot directory +(`/lt-e2e-run-slots`, shared by ALL lt projects): each running suite holds one PID-named +slot file; further runs **wait** (logging every 15s — which also keeps the check-script watchdog +fed, so a queued run is never mistaken for a hang). Crash-safe without a daemon: slots of dead PIDs +are reclaimed by liveness checks. Additionally the vitest config counts foreign slots at load time — +a second run starting seconds after the first deterministically drops to low-resource mode (reduced +forks, raised timeouts), which the 1-minute load average structurally cannot catch. Env knobs: +`LT_E2E_MAX_RUNS` (0 disables), `LT_E2E_SLOT_DIR`, `LT_E2E_SLOT_TIMEOUT` (fail-open). + +## Keep `retry` low — retry multiplies a broken file into a fake deadlock + +Observed with `retry: 5`: one spec file whose app/socket state broke under resource pressure ground +through (1+5) attempts × 30s `testTimeout` × 22 tests ≈ an hour at 0% CPU — indistinguishable from a +deadlocked run (this is what the check-script watchdog kills as "workers idle at 0% CPU"). The +governor removes the pressure trigger; `retry: 2` caps the worst-case multiplier at 3×. Never raise +`retry` to paper over contention — fix the contention. + +## Debug ergonomics + +On a **failed** run the reporter keeps the DBs for inspection — but the data is in the `-w` fork +DBs, not the base. The failure message must list the fork DBs (`${dbName}-w*`), or a developer +connects to the empty base DB. The kept DBs are collected when the **next** run starts (startup +sweep, dead pid) — inspect them before re-running. + +## Checklist when writing / reviewing nest-server e2e tests + +- [ ] `fileParallelism: true`? Then per-worker DB isolation is required if any spec mutates a shared collection. +- [ ] Any `deleteMany({})` / `dropDatabase()` / `.drop()` on a collection another file reads (`jwks`, `users`, `session`, `ratelimitstates`, …)? → isolate per worker, or scope the delete by a per-test filter. +- [ ] `tests/setup.ts` appends `-w` to the **correct** env var (`MONGODB_URI` / `NSC__MONGOOSE__URI`) and is registered in `setupFiles`. +- [ ] `db-lifecycle.reporter.ts` cleanup of other runs' DBs is `isPidAlive` + age guarded (NEVER an unconditional pattern-drop). +- [ ] Per-fork DB names are per-run-unique (carry `-run--p`), not fixed. +- [ ] Failure-path message points at the `-w` fork DBs. +- [ ] Extra per-spec DBs go through `deriveTestDbUri('')` — NEVER `\`-something-${Date.now()}\`` (escapes the per-run cleanup scheme; leaked 2 DBs per run in nest-server until 11.29.0). +- [ ] Spec-level teardown drops its DB **after** `app.close()` — dropping while the app is alive races async module init (AI module collection creation) re-creating the database. +- [ ] `global-setup.ts` runs the startup sweep and acquires an e2e-governor slot; `retry` is ≤ 2. + +## Reference implementation + +`@lenne.tech/nest-server` and `nest-server-starter`: `tests/setup.ts`, `tests/global-setup.ts`, +`tests/db-lifecycle.reporter.ts`, `vitest-e2e.config.ts`. `lt fullstack init` copies these verbatim +from nest-server-starter, so keeping the starter correct fixes new projects automatically; existing +consumers must adopt the pattern. diff --git a/plugins/lt-dev/skills/nest-server-core-vendoring/SKILL.md b/plugins/lt-dev/skills/nest-server-core-vendoring/SKILL.md index da379d5..34694b4 100644 --- a/plugins/lt-dev/skills/nest-server-core-vendoring/SKILL.md +++ b/plugins/lt-dev/skills/nest-server-core-vendoring/SKILL.md @@ -309,6 +309,45 @@ When a local change in the vendored core looks generally useful, the change as upstream-delivered and can remove the local patch from `VENDOR.md`'s local-changes log +**The ceiling is an open DRAFT PR into `develop` — never a merge.** Never merge +the PR, never advance `develop` → `main`, never tag/release/publish. Always +`gh pr create --draft`; never `gh pr ready`. Taking a PR out of draft is how the +maintainer says they have decided — that signal is theirs to give. This holds even +when an earlier instruction sounded like blanket approval ("merge everything"). + +### Upstream branch model (lenneTech/nest-server) + +The default branch is **`develop`**; feature/fix PRs target `develop`. `main` only +receives merge commits (`Merge pull request #NNN from lenneTech/develop`), one per +release. Consequences: + +- **Base contribution branches on `develop`, not `main`.** +- `VENDOR.md` baseline SHAs are **`main` merge commits**, so + `git merge-base --is-ancestor origin/develop` returns **false** even + when the content matches. To find upstream changes since the baseline, diff by + path (`git log ..origin/main -- `) or compare `develop` HEAD + content directly — do not conclude "no changes" from the ancestor check. +- `pnpm run build` regenerates the checked-in `FRAMEWORK-API.md` (timestamp-only + diff). Discard it before committing — it is not part of any fix. + +### Verifying an SWC / circular-import fix + +nest-server ships no `.swcrc`, but `@swc/core` is installed and `npx madge` works. + +- `nest start -b swc` ≈ swc with `module.type=commonjs`, `jsc.target=es2021`, + `legacyDecorator: true`, `decoratorMetadata: true`, `keepClassNames: true`. + Supply that as a config file — **swc defaults alone will NOT reproduce + decorator-driven TDZ crashes**, and you will wrongly conclude there is no bug. +- Compile output must land **inside the repo** (e.g. `./.swc-tdz`), or Node cannot + resolve `@nestjs/common` / `reflect-metadata` from it. SWC keeps the `src/` + prefix → require from `/src/core/...`. +- `git stash -u` swallows untracked probe scripts — write the probe **after** + stashing when A/B-testing baseline vs. patched. +- **Only cycles dereferenced at module-evaluation time are fatal** — e.g. a DI + token inside `@Inject(...)`, which is evaluated when the class is defined. A cycle + that merely touches a class lazily inside a method body is benign. Do not "fix" + those; madge will keep reporting them and that is correct. + ## Cosmetic-vs-Substantial: Why Filter Running `pnpm run format` and `pnpm run lint` on a freshly vendored upstream diff --git a/plugins/lt-dev/skills/nuxt-extensions-core-vendoring/SKILL.md b/plugins/lt-dev/skills/nuxt-extensions-core-vendoring/SKILL.md index 29242b1..b5dcf10 100644 --- a/plugins/lt-dev/skills/nuxt-extensions-core-vendoring/SKILL.md +++ b/plugins/lt-dev/skills/nuxt-extensions-core-vendoring/SKILL.md @@ -263,6 +263,36 @@ When a local change in the vendored core looks generally useful, the change as upstream-delivered and can remove the local patch from `VENDOR.md`'s local-changes log +**The ceiling is an open DRAFT PR into `main` — never a merge.** Never merge the +PR, never tag/release/publish. Always `gh pr create --draft`; never `gh pr ready`. +Taking a PR out of draft is how the maintainer says they have decided — that signal +is theirs to give. This holds even when an earlier instruction sounded like blanket +approval ("merge everything"). + +### Upstream branch model + PR shape (lenneTech/nuxt-extensions) + +This repo has **no `develop` branch** — base every contribution on **`main`**. + +The contribution shape (derived from merged PR #4 and the 1.8.2/1.8.3/1.8.4 release +commits; it is documented nowhere else): + +- **One single commit per PR**, subject = `: ` + (e.g. `1.8.5: accept the nest-server roles: string[] shape in isAdmin …`). +- The commit **bumps `package.json` itself** (patch for a bugfix). +- The commit carries **src + tests only — NO `CHANGELOG.md` entry.** The maintainer + adds it separately as `docs: add changelog entry` on `main` after the + merge. Adding one in the PR duplicates that and causes review churn. +- A pre-commit hook (`.githooks`) auto-runs `oxfmt --write src/` + `oxlint src/`. +- Full gate before committing: `format:check`, `lint`, `dev:prepare`, `test:types`, + `test`, `build` (`pnpm run check:raw` chains them plus `pnpm audit`). + +**Flag the version bump to the user:** if two contribution PRs are open at once, +the second one's bump collides and needs a rebase. + +**Upstream vitest needs `pnpm run dev:prepare` first** (it generates +`.nuxt/tsconfig.json`) — without it every test file fails at transform, which looks +like a broken port but is not. + ## No Reverse Flatten-Fix Needed Unlike backend contributions (where `./common/` must become `./core/common/` diff --git a/plugins/lt-dev/skills/running-check-script/SKILL.md b/plugins/lt-dev/skills/running-check-script/SKILL.md index d3e17f9..2fa79bc 100644 --- a/plugins/lt-dev/skills/running-check-script/SKILL.md +++ b/plugins/lt-dev/skills/running-check-script/SKILL.md @@ -137,6 +137,30 @@ The starter `check` pipeline ends with `bash scripts/check-server-start.sh`, whi These hazards are documented in detail in the `modernizing-toolchain` skill (Phase 6). When a `check` run fails with `ERR_SOCKET_BAD_PORT`, the first triage step is to confirm the script in question already has both the `NITRO_PORT` and ANSI-strip fixes applied. +### Step 6.6 — API tests fail only inside `check`? Suspect a shared test database + +**Symptom:** `check` fails at `api · test` with one or two failures, but the very same suite is **green when run on its own**, and the failing spec **moves between runs** — a missing admin user in one run, a missing season in the next, always in a `prepare` / `beforeAll` step (`Cannot read properties of null (reading '_id')`, `404` on a record the setup just created). + +**This is not flakiness and not load.** It is a second test run — from another working copy — dropping the database out from under this one. The API e2e global setup **drops its database on start**; if two checkouts resolve to the *same* database name, whoever starts second wipes the first one mid-run. Parallel checkouts are routine (`lt ticket` slots, a second clone, another agent session), so this hits often and reads as random. + +**Triage, in this order:** + +1. Run the failing suite **alone**. Green in isolation + wandering failure inside `check` ⇒ this bug, not a real defect. Do **not** "fix" the spec. +2. Check for a competing run: `ps aux | grep [v]itest`. A run in *another* directory of the same project is the smoking gun. +3. Confirm the database name is shared: it must contain a per-run (or at least per-working-copy) component. A constant like `myproject-e2e` is the bug. + +**The fix belongs in the project, not in the spec** — and it very likely already exists upstream. `nest-server` / `nest-server-starter` give every test RUN its own database (`-e2e-run--p`, set in `tests/global-setup.ts` before the workers fork) plus a `db-lifecycle.reporter.ts` that drops it on green, keeps it on red for debugging, and collects the leftovers of crashed runs. A project still on a fixed database name has simply not adopted it — **port the base-repo solution rather than inventing a local one** (see the `contributing-to-lt-framework` skill). + +Things to preserve when porting: + +- **The drop guard.** A test setup that drops "whatever the URI points at" is dangerous, because a running `lt dev` session exports `NSC__MONGOOSE__URI` / `MONGODB_URI` pointing at the **development** database — a suite started in that shell would wipe the developer's data. Refuse to drop any database whose name does not look disposable (`/(e2e|ci|test|acctest)/i`). +- **Derived databases.** A spec that needs its own database must derive it from the run's database (`deriveTestDbUri(...)`), never invent a global name: a fixed name is shared by concurrent runs, and its leftovers are collected by nothing — an aborted run skips the spec's own cleanup, and the databases pile up (one real machine had accumulated ~140). +- **The startup sweep + run governor (11.29.0+).** `tests/global-setup.ts` additionally (a) sweeps stale leftover DBs of this project BEFORE the run (dead-PID/age guarded) — this is what survives SIGKILL and `--reporter` overrides, and (b) acquires a machine-wide e2e slot (`tests/e2e-run-slots.ts`, `/lt-e2e-run-slots`) so concurrent `check` runs from parallel sessions queue instead of starving each other. **A run printing `[e2e-governor] waiting for a free e2e slot` every 15s is QUEUED, not hung** — do not kill it and do not misread it as a deadlock. The config also drops to low-resource mode (reduced forks) when another run is active. + +### Step 6.7 — `api · test` looks deadlocked (0% CPU, no output)? It is usually a retry grind + +A spec file whose app/socket state broke (historically: resource starvation from overlapping runs) grinds through `(1+retry)` attempts × `testTimeout` × tests-per-file — with `retry: 5` that was up to an hour at 0% CPU, which the watchdog kills as "workers idle". Base repos now ship `retry: 2` and the run governor removes the starvation trigger. If you still see it: check `pgrep -f vitest` for a single surviving fork at 0% CPU, read which spec file last logged, and re-run that file alone. Never raise `retry` to paper over it. + ### Step 7 — Test-duplication avoidance Tests must not run twice if `check` already covered them on an unchanged working tree. diff --git a/plugins/lt-offers/.claude-plugin/plugin.json b/plugins/lt-offers/.claude-plugin/plugin.json index f03012c..1612337 100644 --- a/plugins/lt-offers/.claude-plugin/plugin.json +++ b/plugins/lt-offers/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "lt-offers", - "version": "7.57.0", + "version": "7.58.0", "description": "Skills & Commands for creating and managing business offers on angebote.lenne.tech (production) and demo-angebote.lenne.tech (demo) — MCP tools for offers, knowledge base, analytics, and content management", "author": { "name": "lenne.tech GmbH", diff --git a/plugins/lt-showroom/.claude-plugin/plugin.json b/plugins/lt-showroom/.claude-plugin/plugin.json index ca63413..cd0c1c9 100644 --- a/plugins/lt-showroom/.claude-plugin/plugin.json +++ b/plugins/lt-showroom/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "lt-showroom", - "version": "7.57.0", + "version": "7.58.0", "description": "Skills & Commands for analyzing software projects and creating showcases on showroom.lenne.tech — MCP tools for showcases, screenshots, analytics, and content management", "author": { "name": "lenne.tech GmbH",