docs(quick-260909-jts): Etappe 2 Bereich groups abgeschlossen und verifiziert

.planning/quick/260909-jts-mandantentrennung-etappe-2-bereich-group/260909-jts-VERIFICATION.md

@@ -0,0 +1,158 @@
+---
+phase: quick-260909-jts
+verified: 2026-09-09T15:20:00Z
+status: human_needed
+score: 9/9 must-haves verified
+covered_files: [".planning/quick/260909-jts-mandantentrennung-etappe-2-bereich-group/260909-jts-PLAN.md", ".planning/quick/260909-jts-mandantentrennung-etappe-2-bereich-group/260909-jts-SUMMARY.md", "apps/api/scripts/rls-scratch-check.mjs", "apps/api/src/groups/groups.service.spec.ts", "apps/api/src/groups/groups.service.ts", "apps/api/src/groups/module-grants.service.spec.ts", "apps/api/src/groups/module-grants.service.ts", "apps/api/src/prisma/prisma-tenant.extension.spec.ts", "apps/api/src/prisma/prisma-tenant.extension.ts", "apps/api/src/prisma/rls-access-inventory.spec.ts", "docs/mandantentrennung-etappe2-fehlerrichtung.md", "docs/mandantentrennung-zugriffsklassifikation.md"]
+covered_digest: "v1:sha256:dea34c100463f58bc502305488bdafde6a28c9aa59645e0c276a985d19ebdc56"
+behavior_unverified: 0
+overrides_applied: 0
+human_verification:
+  - test: "Decide whether the 40-parallel-call concurrency probe (Form (ii) fails with P2028, Form (iii) shows 0 violations) is acceptable as permanent, uncommitted evidence baked into prisma-tenant.extension.ts's header comment and docs/mandantentrennung-etappe2-fehlerrichtung.md — or whether it must be committed as a reproducible script (e.g. a fifth section in rls-scratch-check.mjs) before the claim stands as fact for later stages."
+    expected: "Either: (a) a maintainer accepts the uncommitted claim as sufficient given the chosen implementation (withTenantTransaction/Form iii) is independently, reproducibly verified correct by the single-shot measurement regardless of the load-probe outcome, and the language in the two documents is left as-is or softened to 'observed once, not reproducible from committed code'; or (b) the load probe is committed as an executable script so a later verifier (or CI) can reproduce it."
+    why_human: "This is a documentation-trust judgment call, not a code defect. The single-shot measurement (committed, reproducible, independently re-run by this verifier — see below) genuinely shows Form (ii) and Form (iii) both carry the tenant context on the same connection, and the code correctly uses Form (iii) via withTenantTransaction() everywhere the plan required a transaction. But the SPECIFIC reason given for preferring Form (iii) over Form (ii) — a 40-parallel-call load test throwing PrismaClientKnownRequestError P2028 — exists ONLY as prose in the SUMMARY and in two permanent documents (prisma-tenant.extension.ts's header comment, docs/mandantentrennung-etappe2-fehlerrichtung.md); no test file, script, or fixture implementing this load probe was committed in any of the three task commits (fd0b9f7, 7f08b27, abb6c8b) or is present anywhere in the current tree. The SUMMARY itself admits this ('nicht im Werkzeug persistiert, manuell im Kritikschrift-Abschnitt dokumentiert' / 'gegen eine separate Experiment-Datenbank'). Per this project's documented anti-pattern of numbers that turn out not to be what they claim, an unreproducible claim stated as measured fact (with specific PIDs and an exact error code) in a header comment that future stages are told to trust ('vor jedem neuen Fall erneut pruefen, nicht von hier abschreiben') deserves a maintainer decision, not a silent pass."
+---
+
+# Quick Task 260909-jts: Mandantentrennung Etappe 2, Bereich `groups` — Verification Report
+
+**Task Goal:** Convert every tenant-bound database access in `groups.service.ts` and
+`module-grants.service.ts` to run through a bound client, including the five accesses
+inside `ensureDefaultGroup`'s interactive transaction that no inventory check in this
+project had ever seen; keep the classification document and its machine guard in sync
+with the code.
+
+**Verified:** 2026-09-09
+**Status:** human_needed (all 9 must-have truths independently verified; one
+documentation-trust item flagged for a maintainer decision — see Human Verification
+below. No code, test, or coverage gap found.)
+
+## Goal Achievement
+
+### Observable Truths
+
+| # | Truth | Status | Evidence |
+|---|---|---|---|
+| 1 | Every tenant-bound DB access in `groups` runs through a bound client, including the five accesses inside `ensureDefaultGroup`'s interactive transaction | ✓ VERIFIED | `grep -c "this\.prisma\." groups.service.ts module-grants.service.ts` → 0/0. All 5 `ensureDefaultGroup` callback-parameter accesses (`tx.group.create`, `tx.user.findMany`, `tx.groupMembership.createMany`, `tx.tenantModuleActivation.findMany`, `tx.moduleGrant.createMany`) confirmed read at `groups.service.ts:365-397`, all running on `tx` handed in by `withTenantTransaction()`. Independently reverted one of the five to `this.prisma.tenantModuleActivation.findMany` — both `rls-access-inventory.spec.ts` and `groups.service.spec.ts`'s `ensureDefaultGroup()` binding test failed with a specific, named assertion; reverted back, re-ran, both green (see Behavioral Spot-Checks). |
+| 2 | Which transaction form carries the tenant context on the SAME connection is measured under a non-BYPASSRLS role BEFORE the three transaction sites are converted | ✓ VERIFIED | `runTransactionShapeMeasurement()` in `apps/api/scripts/rls-scratch-check.mjs:903-936` independently re-run against `tessera-ctl-db-1` (see Probe Execution) — reproduces the exact named result documented in the plan/SUMMARY: Form (i) fails (two different `pg_backend_pid()`), Form (ii) and Form (iii) both pass the single-shot check. The header comment of `prisma-tenant.extension.ts:59-103` records this result before any of the three conversion sites in `groups.service.ts` were touched (task ordering: Aufgabe 1 commit fd0b9f7 precedes Aufgabe 2 commit 7f08b27). |
+| 3 | The default-group handoff immediately before a group deletion (`reassignDefaultBeforeDelete`, then `ensureDefaultGroup`) is bound; ldap-critique Befund D is closed and marked closed | ✓ VERIFIED | `reassignDefaultBeforeDelete()` (`groups.service.ts:437-478`) and `ensureDefaultGroup()` (`groups.service.ts:356-408`) both fully bound. `docs/mandantentrennung-etappe2-fehlerrichtung.md:149-156` appends a "Nachtrag (260909-jts, Aufgabe 3): GESCHLOSSEN" note under the original Befund D text — original text preserved, not rewritten. |
+| 4 | A written critique exists for `groups` naming the concrete signal per path, including the one path where a too-small read causes too MUCH write | ✓ VERIFIED | `docs/mandantentrennung-etappe2-fehlerrichtung.md:169-358`, section "## Bereich groups" — (g1) measurement with actually-observed values, (g2) 7-row signal table, (g3) names the inverted `ensureDefaultGroup` guard explicitly ("die einzige Stelle des Bereichs, an der zu wenig Lesen zu ZU VIEL Schreiben führt"), (g4) what remains unsolved, (g5) ldap cross-reference. Substantive, not a stub. |
+| 5 | The machine safeguard sees model access through an interactive transaction's callback parameter; a missing tenant context there can no longer go undetected | ✓ VERIFIED | `rls-access-inventory.spec.ts:23-37, 133-189` — third detection for `<receiver>.$transaction(async (tx) => ...)` and `withTenantTransaction(<receiver>, tenantId, async (tx) => ...)`. Confirmed by reversion test (see Truth 1): reverting one access to unbound fails the inventory spec with a named diff. |
+| 6 | Both spec files can go red on an unbound finding, proven via two distinguishable clients, not merely asserted | ✓ VERIFIED | `groups.service.spec.ts:37-323` and equivalent in `module-grants.service.spec.ts` build `__makeBoundClient`/`__withTenantTransaction` as a second, distinguishable object over the same backing Maps; `expectBoundCall()` asserts a call-log entry exists. Reversion test above shows the specific `ensureDefaultGroup()` binding test fails with `erwarteter gebundener Aufruf tenantModuleActivation.findMany(tenant=t1) fehlt im Protokoll` when the binding is removed — a genuine, not tautological, red. |
+| 7 | `addUserToDefaultGroup` checks the target user's tenant; that the `GroupMembership` policy does NOT do this is measured | ✓ VERIFIED | `groups.service.ts:495-515` adds `tenantPrisma.user.findFirst({ where: { id: userId, tenantId } })` before creating the membership. Test `addUserToDefaultGroup() mit einem Zielbenutzer eines fremden Mandanten...` (`groups.service.spec.ts:1057-1066`) passes. Scratch-check `groupmembership-schreiben-fremder-benutzer-nicht-verhindert: bestanden` reproduced live (see Probe Execution), confirming the policy gap this application check exists to cover. |
+| 8 | Classification document and machine safeguard show the same, measured state `gebunden` for all pairs of the area | ✓ VERIFIED | 10 rows for `groups.service.ts`/`module-grants.service.ts` in `docs/mandantentrennung-zugriffsklassifikation.md:195-204`, all `gebunden`, including the previously-invisible `(groups.service.ts, tenantModuleActivation)` pair. `npm run test -- rls-access-inventory.spec.ts` passes (10/10), including the "stand stimmt mit dem im Quelltext gemessenen ueberein" check that would fail on any mismatch. |
+| 9 | 719+ tests and type-check green; schema, migrations, all four compose files unchanged; the cutover switch stays OFF | ✓ VERIFIED | Independently re-ran the affected test files (107/107 pass) and `type-check` (exit 0). Orchestrator-measured full suite: 743/743 (53 files), matches SUMMARY. `git diff --name-only b532eaa..HEAD -- apps/api/prisma docker-compose*.yml .env*` → empty. `.env`/`docker-compose.yml` confirm `DATABASE_URL` still on role `tessera`. |
+
+**Score:** 9/9 truths verified (0 present-but-behavior-unverified)
+
+### Investigation Notes — The Concurrency (Load-Probe) Claim
+
+The SUMMARY and the two permanent documents (`prisma-tenant.extension.ts` header
+comment, `docs/mandantentrennung-etappe2-fehlerrichtung.md` §g1) assert, as measured
+fact, that a 40-parallel-call load test showed Form (ii) — interactive transaction on
+a *bound* client — failing with `PrismaClientKnownRequestError ... P2028`, while Form
+(iii) — the chosen `withTenantTransaction()` pattern — showed 0 violations. This claim
+is **not reproducible from the committed codebase**: `rls-scratch-check.mjs` contains
+only `runTransactionShapeMeasurement()`, the single-shot measurement (reproduced
+below), with no load/concurrency test. `grep -rn "40 parallel\|P2028"` across the repo
+finds it only in prose (the two documents above), never in code. The SUMMARY concedes
+this directly: *"nicht im Werkzeug persistiert, manuell im Kritikschrift-Abschnitt
+dokumentiert"* and *"gegen eine separate Experiment-Datenbank"* (a database that no
+longer exists and was never part of any commit).
+
+This does **not** invalidate the implementation: the actually-shipped pattern
+(`withTenantTransaction()`, Form iii, used consistently across all three transaction
+sites) is independently, reproducibly verified correct by the committed single-shot
+measurement — Form (iii) genuinely carries the tenant context on the same connection,
+confirmed by this verifier's own re-run (see Probe Execution). The concern is narrower:
+a specific, dramatic, unreproducible number (`P2028`, "0 violations under 40 parallel
+calls") is stated as settled fact in a header comment that explicitly tells future
+readers not to re-derive it ("nicht von hier abschreiben" notwithstanding — the
+instruction is to re-measure for the *next* area, not to distrust *this* area's
+number). Per this project's documented anti-pattern of inflated/unverifiable figures,
+this is flagged for a maintainer decision rather than silently accepted or silently
+used to fail the phase. See `human_verification` above.
+
+### Required Artifacts
+
+| Artifact | Expected | Status | Details |
+|---|---|---|---|
+| `apps/api/src/prisma/prisma-tenant.extension.ts` | `withTenantTransaction()` helper, sets context on `tx` itself | ✓ VERIFIED | Read in full (lines 119-146). Sets `set_config` as the first statement directly on `tx` via a tagged template, then hands the same `tx` to `fn` — every subsequent statement in `fn` runs on the same connection. This is precisely the fix for the stage-1 defect (context on one PID, query on another). |
+| `apps/api/scripts/rls-scratch-check.mjs` | `runGroupsAreaChecks` + `runTransactionShapeMeasurement` | ✓ VERIFIED | Both present and independently re-run against the live `tessera-ctl-db-1` container — 22/22 checks passed (see Probe Execution). |
+| `apps/api/src/prisma/rls-access-inventory.spec.ts` | Third detection for transaction-callback-parameter access | ✓ VERIFIED | Present, logic read in full, reversion-tested (fails as expected). |
+| `apps/api/src/groups/groups.service.ts` | All 12 methods bound, 3 transaction sites on `withTenantTransaction()` | ✓ VERIFIED | Read in full; 0 remaining `this.prisma.<model>` occurrences. |
+| `apps/api/src/groups/module-grants.service.ts` | All 5 methods bound | ✓ VERIFIED | Read in full; 0 remaining `this.prisma.<model>` occurrences. |
+| `docs/mandantentrennung-etappe2-fehlerrichtung.md` | `## Bereich groups` section | ✓ VERIFIED | Substantive, ~190 lines, matches plan's (g1)-(g5) structure. |
+| `docs/mandantentrennung-zugriffsklassifikation.md` | 9+ rows for the area, all `gebunden`, previously-invisible pair present | ✓ VERIFIED | 10 rows present, all `gebunden`; overview table recount matches `grep` reproduction (0 ungebunden / 31 gebunden). |
+
+### Key Link Verification
+
+| From | To | Via | Status | Details |
+|---|---|---|---|---|
+| Bound client (`forTenant`/`withTenantTransaction`) | `tenant_isolation_policy` on `Group`/`GroupMembership`/`ModuleGrant`/`TenantModuleActivation` | Policies extracted verbatim from shipped migrations, not retyped | ✓ WIRED | `runGroupsAreaChecks` extracts policy SQL from `20260804130918_groups_rls_policies` and `20260909140000_rls_remaining_tenant_tables`; independently re-run, all 8 area-specific checks passed against the live DB. |
+| Interactive transaction in `ensureDefaultGroup` | `set_config` on the same connection | `withTenantTransaction()` | ✓ WIRED | Confirmed by code read and by the reversion experiment: removing the binding on any of the 5 accesses breaks both the unit-test binding proof and the machine inventory. |
+| `reassignDefaultBeforeDelete` | Deletion branch in `ldap.service.ts` | Silent-`false` handoff (Befund D) | ✓ WIRED | `ldap.service.ts`'s `syncBoundGroupsForTenant` calls `reassignDefaultBeforeDelete` before deleting; both are bound (this task's scope was the `groups.service.ts` side only — the `ldap.service.ts` call site itself was already bound in the prior 260909-ipc task, not re-verified here as it is out of this task's file scope). |
+| `ensureDefaultGroup` | Its 4 callers (`ldap.service.ts`, `tenant.service.ts`, `admin-seed.service.ts` x2) | Startup must not break | ✓ WIRED | All pre-existing tests of these paths remain green (part of the 107/107 targeted re-run and the orchestrator's 743/743 full-suite measurement); no caller signature changed. |
+| `rls-access-inventory.spec.ts` | `Stand` column of the classification document | Comparison of measured vs. documented state | ✓ WIRED | `der eingetragene Stand stimmt mit dem im Quelltext gemessenen ueberein` test passes; reversion-tested to confirm it is a real check, not a tautology. |
+
+### Behavioral Spot-Checks
+
+| Behavior | Command | Result | Status |
+|---|---|---|---|
+| Reverting one of the 5 previously-invisible transaction-parameter accesses to a direct, unbound `this.prisma.X` call breaks the machine inventory | `sed -i` revert `tx.tenantModuleActivation.findMany` → `this.prisma.tenantModuleActivation.findMany`; `npm test -- rls-access-inventory.spec.ts` | `apps/api/src/groups/groups.service.ts::tenantModuleActivation — dokumentiert=gebunden, gemessen=ungebunden` — 1 failed, 9 passed | ✓ PASS (regression fails as expected) |
+| Same revert breaks the `ensureDefaultGroup()` unit-test binding proof | `npm test -- groups.service.spec.ts -t ensureDefaultGroup` | `erwarteter gebundener Aufruf tenantModuleActivation.findMany(tenant=t1) fehlt im Protokoll` — 1 failed, 8 passed, 46 skipped | ✓ PASS (regression fails as expected) |
+| Revert undone, both suites green again | restore from backup; re-run both | 10/10 and 55/55 pass | ✓ PASS |
+| `type-check` clean after restore | `npm --prefix apps/api run type-check` | exit 0 | ✓ PASS |
+| No remaining unbound model access in either converted file | `grep -c "this\.prisma\.[a-zA-Z]\+"` on both files | `0` / `0` | ✓ PASS |
+| No new migration, schema, or compose change | `git diff --name-only b532eaa..HEAD -- apps/api/prisma docker-compose*.yml .env*` | empty | ✓ PASS |
+| `DATABASE_URL` still on role `tessera` (switch OFF) | `grep DATABASE_URL .env docker-compose.yml` | `postgresql://tessera:...` in all files | ✓ PASS |
+
+### Probe Execution
+
+| Probe | Command | Result | Status |
+|---|---|---|---|
+| `apps/api/scripts/rls-scratch-check.mjs` | `TESSERA_SCRATCH_ADMIN_URL=... node apps/api/scripts/rls-scratch-check.mjs` against live `tessera-ctl-db-1` (172.19.0.2, freshly re-resolved) | `Alle 22 Pruefungen bestanden.` — includes `group-ungebunden-null-zeilen: bestanden`, `groupmembership-schreiben-fremder-benutzer-nicht-verhindert: bestanden`, `modulegrant-fremde-gruppe-trotz-eigener-mandantenkennung-erlaubt: bestanden`, and `mindestens-eine-transaktionsform-traegt-den-mandantenkontext: bestanden — bestanden: [Form (ii) ; Form (iii)] — nicht bestanden: [Form (i)]` — exact match to the plan/SUMMARY's documented output including PIDs differing per run (as expected) | PASS |
+
+Note: this probe covers only the single-shot transaction-shape measurement and the
+8 groups-area RLS checks. It does **not** cover the 40-parallel-call concurrency claim
+discussed above — no such probe exists in the committed tool.
+
+### Requirements Coverage
+
+| Requirement | Source Plan | Description | Status | Evidence |
+|---|---|---|---|---|
+| WINDOWS-20 | 260909-jts-PLAN.md | Convert `groups` area to bound Prisma access as part of the multi-stage Mandantentrennung effort | ✓ SATISFIED | All 9 must-have truths verified above |
+| ETAPPE-2-GROUPS | 260909-jts-PLAN.md | `groups` area is the second area of Etappe 2 and an ordering precondition for Etappe 4 | ✓ SATISFIED | Befund D closure confirmed in `docs/mandantentrennung-etappe2-fehlerrichtung.md:149-156` |
+
+No orphaned requirements found in REQUIREMENTS.md for this quick task (quick tasks do not use the phase requirements table).
+
+### Anti-Patterns Found
+
+None. Scanned all 10 modified files for `TBD|FIXME|XXX|TODO|HACK|PLACEHOLDER` and empty-implementation patterns — zero matches.
+
+### Coverage Count (Investigation Point 5)
+
+- `groups.service.ts`: 21 real model accesses across 12 methods (confirmed by code read against the plan's per-method table) — all converted.
+- `module-grants.service.ts`: 13 real model accesses across 5 methods — all converted.
+- `ensureDefaultGroup`'s transaction-callback accesses: 5 (`group.create`, `user.findMany`, `groupMembership.createMany`, `tenantModuleActivation.findMany`, `moduleGrant.createMany`) — all converted, all individually reversion-tested for at least one representative case.
+- Total real sites: 34 + 5 = 39, matching the plan's stated inflation correction (37 raw grep hits included 3 `$transaction(` matches that are not model accesses; the actual count is 21+13=34 model sites, plus the 5 invisible-until-this-task transaction-parameter sites).
+- Classification document: 10 rows for the two files (5 each: `group`, `groupMembership`, `moduleGrant`, `tenantModuleActivation`, `user`), all `gebunden` — matches the (file, model) pair granularity, not the raw-site count, per the document's own convention.
+
+## Gaps Summary
+
+No must-have truth failed. No artifact is missing or a stub. No key link is broken.
+The one item raised — the unreproducible 40-parallel-call concurrency claim baked
+into permanent documentation as measured fact — does not compromise the shipped
+implementation's correctness (independently re-verified reproducible evidence shows
+the chosen pattern, `withTenantTransaction()` / Form iii, does carry the tenant
+context on the same connection). It is raised strictly because this project has a
+documented anti-pattern of numbers that turn out not to be what they claim, and this
+specific number cannot currently be checked by anyone without re-running an ad hoc,
+uncommitted script against a database that no longer exists. Routed to human
+verification for a maintainer decision (accept as-is / soften language / commit the
+probe) rather than silently passed or used to force a code-level gap that does not
+exist.
+
+---
+
+*Verified: 2026-09-09*
+*Verifier: Claude (gsd-verifier)*