From 0379016c7e11fd5e2ee6a0d47db15a0d01582cc2 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Wed, 6 May 2026 13:03:00 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20WS-7=20PR-2=20follow-up=20=E2=80=94=20R?= =?UTF-8?q?FC=20=C2=A73.6=20+=20=C2=A73.14=20+=20BACKLOG=20OBS=20entries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC §3.6 — context tagging tabel volledig vervangen na de PR-2 follow-up architecturale fixes. Belangrijkste wijzigingen: - Tag-binding gesplitst in route-scope (BindSentryRouteContext middleware) en auth-scope (AuthScopeContextListener op Authenticated event). - Nieuwe actor_scope tag (organisation/platform/user/anonymous). - Multi-tenant invariant verfijnd: organisation_id is altijd correct gerelateerd aan actor_scope in plaats van "altijd aanwezig". Platform- routes zonder org-context worden niet meer gefabriceerd; default authenticated user-scope omitt organisation_id (Crewli's User<->Organisation is many-to-many, geen reliable single-org hint). - impersonation.* tags expliciet gedocumenteerd als afkomstig uit HandleImpersonation middleware (post-swap), niet uit auth-listener. - ActorType waarden bijgewerkt na verwijdering van VOLUNTEER case. RFC §3.14 — status-note toegevoegd dat D-06 indexes al via Spatie's nullableMorphs default-migratie zijn aangemaakt, met regression-guard verwijzing. §6 acceptance criterium 12 markeert D-06 als al voldaan. BACKLOG.md krijgt vier nieuwe OBS-entries: - OBS-1: VOLUNTEER actor_type promotion wanneer rol komt - OBS-4: PHPUnit metadata deprecation cleanup pre-PHPUnit-12 - OBS-6: sentry-laravel install gap awareness + bootstrap test - OBS-7: custom render handlers report() invariant + coverage Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Observability/ExceptionReportingTest.php | 4 +- dev-docs/BACKLOG.md | 104 +++++++++++++++++- dev-docs/RFC-WS-7-OBSERVABILITY.md | 51 ++++++--- 3 files changed, 139 insertions(+), 20 deletions(-) diff --git a/api/tests/Feature/Observability/ExceptionReportingTest.php b/api/tests/Feature/Observability/ExceptionReportingTest.php index 05865986..1c3fda9a 100644 --- a/api/tests/Feature/Observability/ExceptionReportingTest.php +++ b/api/tests/Feature/Observability/ExceptionReportingTest.php @@ -147,8 +147,8 @@ final class ExceptionReportingTest extends TestCase $this->assertCount(1, self::$captured); $tags = self::$captured[0]['event']->getTags(); - // BindSentryContext should have set these on the scope before - // the exception fired in the controller. + // BindSentryRouteContext should have set these on the scope + // before the exception fired in the controller. $this->assertSame('api', $tags['app'] ?? null); $this->assertSame('GET', $tags['http.method'] ?? null); } diff --git a/dev-docs/BACKLOG.md b/dev-docs/BACKLOG.md index 52386777..f3c5c4e4 100644 --- a/dev-docs/BACKLOG.md +++ b/dev-docs/BACKLOG.md @@ -1638,5 +1638,105 @@ voeden). --- -_Laatste update: April 2026_ -_Voeg nieuwe items toe met prefix: ARCH-, COMM-, OPS-, VOL-, ART-, FORM-, SUP-, DIFF-, APPS-, TECH-, UX-_ +_Laatste update: Mei 2026_ +_Voeg nieuwe items toe met prefix: ARCH-, COMM-, OPS-, VOL-, ART-, FORM-, SUP-, DIFF-, APPS-, TECH-, UX-, OBS-_ + +--- + +## Observability follow-ups (post WS-7 PR-2) + +### OBS-1 — Promote ActorType::VOLUNTEER when volunteer role is introduced + +**Aanleiding:** WS-7 PR-2 architectural-fix-commit verwijderde de +`ActorType::VOLUNTEER` enum-case omdat Crewli vandaag geen dedicated +`volunteer` Spatie-rol heeft — vrijwilligers zijn behaviorally bepaald +(users met shift-assignments), niet identitair. De resolver mapt +non-admin authenticated users naar `ORG_MEMBER`. + +**Wat:** Wanneer Crewli een `volunteer` rol invoert (bijv. via +volunteer-onboarding workflow), her-introduceer dan de `VOLUNTEER` +case in `app/Enums/Observability/ActorType.php` en update +`ActorType::resolve()` om de rol te checken vóór `ORG_MEMBER`. Update +ook `AuthScopeContextListenerTest` met een `actor_type=volunteer` +testcase. + +**Prioriteit:** Laag — wachten op een product-besluit over volunteer-rol +modellering. Geen blocker. + +**Refs:** `app/Enums/Observability/ActorType.php`, +RFC-WS-7-OBSERVABILITY.md §3.6. + +### OBS-4 — PHPUnit metadata-in-doc-comment deprecation cleanup + +**Aanleiding:** PHPUnit warnt dat metadata in doc-comments (zoals +`@test`, `@dataProvider`) deprecated is en in PHPUnit 12 verwijderd +wordt. Crewli heeft drie tests met deze pattern: + +- `Tests\Unit\Support\Json\JsonCanonicalizerTest::test_scalar_passthrough()` +- `Tests\Feature\FormBuilder\Purposes\PurposeSchemaLifecycleTest::test_create_and_publish_succeeds_for_purpose()` +- `Tests\Feature\Schema\UlidPrimaryKeyTest::test_model_uses_has_ulids_and_generates_crockford_ulid()` + +**Wat:** Vervang de doc-comment metadata door PHPUnit attributes +(bijv. `#[Test]`, `#[DataProvider]`). Raak alleen aan vóór de PHPUnit 12 +upgrade gepland wordt — nu blokkeert het niets. + +**Prioriteit:** Laag — kosmetisch totdat PHPUnit 12 upgrade landt. + +**Refs:** PHPUnit changelog, de drie genoemde test-files. + +### OBS-6 — sentry-laravel installation gap awareness + +**Aanleiding:** WS-7 PR-2 smoke test faalde silent omdat sentry-laravel +4.x de `Integration::handles($exceptions)` registratie niet +auto-registreert in zijn ServiceProvider. De host-app moet de regel +expliciet aan `bootstrap/app.php` toevoegen. README documenteert dit, +maar tijdens `composer require sentry/sentry-laravel` + +`php artisan sentry:publish` workflow is het makkelijk te missen. + +**Wat:** + +- Voeg een waarschuwing toe in `dev-docs/SETUP.md` onder een nieuwe + sectie "Laravel package installation patterns": bij elke nieuwe + package altijd verifiëren dat het package zijn ServiceProvider- + registraties doet voor exception handlers, queue listeners, en log + channels — niet alleen voor routes/views/migrations. +- Overweeg een `tests/Feature/Bootstrap/ExceptionHandlerRegistrationTest.php` + die `app(\Illuminate\Foundation\Exceptions\Handler::class)->getReportableCallbacks()` + introspecteert en assertert dat sentry-laravel's callback + geregistreerd is. Vangt een toekomstige refactor die per ongeluk + `Integration::handles` uit `bootstrap/app.php` verwijdert. + +**Prioriteit:** Laag — fix is gedaan en getest, regression mogelijk +maar onwaarschijnlijk gezien de explicit comment in `bootstrap/app.php`. + +**Refs:** `bootstrap/app.php`, +`vendor/sentry/sentry-laravel/src/Sentry/Laravel/Integration.php`, +RFC-WS-7-OBSERVABILITY.md §3.10. + +### OBS-7 — Custom $exceptions->render() handlers report() invariant + +**Aanleiding:** WS-7 PR-2 smoke-test debugging onthulde dat Crewli's +`bootstrap/app.php` 5 custom render handlers heeft. Met +`Integration::handles($exceptions)` geregistreerd werkt +report-before-render correct. Maar een toekomstige render handler die +een Throwable consumeert zonder `report($e)` aan te roepen vóór return +zou Sentry-capture kunnen overslaan voor die exception class. + +**Wat:** + +- Documenteer in `bootstrap/app.php` (boven het withExceptions block) + een comment: "Render handlers consume exceptions; Laravel's + ExceptionHandler::handle() doet report() vóór render() zodat capture + automatisch is. NIEUWE render handlers MOGEN NIET short-circuiten + voordat report() bereikt is. Verifieer via + tests/Feature/Observability/ExceptionReportingTest.php." +- Uitbreiden van `ExceptionReportingTest.php` met assertions per + bestaande render handler class: throw die exception, assert event + captured. + +**Prioriteit:** Medium — bestaande handlers zijn correct, maar het +invariant is subtiel en silent-failure-prone bij toevoegingen. + +**Refs:** `bootstrap/app.php`, +`tests/Feature/Observability/ExceptionReportingTest.php`, +RFC-WS-7-OBSERVABILITY.md §3.10. diff --git a/dev-docs/RFC-WS-7-OBSERVABILITY.md b/dev-docs/RFC-WS-7-OBSERVABILITY.md index 1ed75cdf..2d0cb0ba 100644 --- a/dev-docs/RFC-WS-7-OBSERVABILITY.md +++ b/dev-docs/RFC-WS-7-OBSERVABILITY.md @@ -61,24 +61,41 @@ Format `@` (`crewli-api@f41951a`, `crewli-app@f41951a`). Bron: ` ### 3.6 Context tagging -| Tag | API | apps/app | -|---|---|---| -| `release` | altijd | altijd | -| `environment` | altijd | altijd | -| `app` | `api` | `app` | -| `route_name` | `Route::currentRouteName()` | `route.name` | -| `http.method` | altijd | n.v.t. | -| `organisation_id` (ULID) | wanneer auth+scope gebound | uit auth store | -| `event_id` (ULID) | wanneer event-scoped | wanneer applicabel | -| `user_id` (ULID) | `auth()?->id()` | uit auth store, alleen session-mode | -| `actor_type` | `organizer_admin` / `super_admin` / `portal_token` / `volunteer` / etc. | mirror | -| `impersonation.active` | bool | n.v.t. | -| `impersonation.impersonator_user_id` | wanneer actief | n.v.t. | -| `queue.attempt` | binnen job-context | n.v.t. | +Tag-binding gebeurt op twee plekken: route-scope tags via `BindSentryRouteContext` middleware (op de api-group), auth-scope tags via `AuthScopeContextListener` op `Illuminate\Auth\Events\Authenticated`. De split volgt de data-bron: route-context is alleen tijdens HTTP-handling beschikbaar, auth-context wordt door elke authenticator (Sanctum, portal-token) ge-emit via het Authenticated event. + +| Tag | API | apps/app | Bron / locatie | +|---|---|---|---| +| `release` | altijd | altijd | env, sentry-laravel built-in | +| `environment` | altijd | altijd | env, sentry-laravel built-in | +| `app` | `api` | `app` | route-middleware | +| `route_name` | altijd | altijd | route-middleware | +| `http.method` | altijd | n.v.t. | route-middleware | +| `actor_scope` | `organisation`/`platform`/`user`/`anonymous` | mirror | auth-listener (zie hieronder) | +| `organisation_id` (ULID) | aanwezig wanneer `actor_scope = organisation` | uit auth store | auth-listener | +| `event_id` (ULID) | wanneer event-scoped | wanneer applicabel | auth-listener (via {event} route-param) | +| `user_id` (ULID) | wanneer authenticated | uit auth store, alleen session-mode | auth-listener | +| `actor_type` | `organizer_admin` / `super_admin` / `portal_token` / `org_member` / `unauthenticated` | mirror | auth-listener | +| `impersonation.active` | bool | n.v.t. | HandleImpersonation middleware (post-swap) | +| `impersonation.impersonator_user_id` | wanneer actief | n.v.t. | HandleImpersonation middleware | +| `impersonation.session_id` | wanneer actief | n.v.t. | HandleImpersonation middleware | +| `queue.attempt` | binnen job-context | n.v.t. | TagJobAttemptOnSentry listener | **Nooit als tag:** email, telefoon, naam, IP-adres, raw form_value content, raw cookie content. -Multi-tenant invariant: élke captured event uit een geauthenticeerde controller MOET `organisation_id` hebben. Een unit-test verifieert dit — als `organisation_id` ontbreekt op een geauthenticeerd path, faalt de test. +**Multi-tenant invariant (verfijnd na PR-2 live smoke test):** + +`actor_scope` is altijd aanwezig op authenticated events. Wanneer `actor_scope = organisation`, MOET `organisation_id` aanwezig en valide ULID zijn. Wanneer `actor_scope = platform`, IS `organisation_id` afwezig — dat is correct gedrag voor super_admin platform-routes (geforceerde org-attribution zou misleidend zijn). Wanneer `actor_scope = user` (default authenticated zonder org-route-context), is `organisation_id` ook afwezig: Crewli's User↔Organisation is many-to-many, een single-org "current org" bestaat niet op user-niveau, en attribution aan een willekeurige org zou misleiden. Een unit-test in `AuthScopeContextListenerTest::test_organisation_id_present_when_actor_scope_is_organisation` verifieert deze invariant. + +**`actor_scope`-waarden:** + +| Waarde | Wanneer | Filtering use-case in GlitchTip | +|---|---|---| +| `organisation` | route met {organisation} of {event} param, of portal-token request | "Issues voor organisatie X" | +| `platform` | super_admin op `admin.*` named routes | "Platform-bugs (niet org-specifiek)" | +| `user` | authenticated user op routes zonder org-scope (`/me/*`, `/portal/my-shifts`, `/uploads/*` etc.) | "Issues op user-routes; geen org-attribution" | +| `anonymous` | unauthenticated requests | "Public-route issues" | + +**Wijziging t.o.v. originele RFC:** de oorspronkelijke formulering "élke captured event uit een geauthenticeerde controller MOET `organisation_id` hebben" is verfijnd na bevinding dat super_admin platform-routes geen zinvolle org-context hebben en Crewli's many-to-many user-org model geen reliable single-org hint biedt. Het invariant is nu sterker: niet "altijd aanwezig" maar "altijd correct gerelateerd aan `actor_scope`." ### 3.7 PII scrubbing @@ -143,6 +160,8 @@ Log::withContext([ `(subject_type, subject_id)` en `(causer_type, causer_id)` composite indexes op `activity_log`. Infrastructure-housekeeping; geen functionele wijziging. +**Status (mei 2026, na PR-2):** Bij implementatie bleek dat de Spatie activitylog default-migratie via `nullableMorphs('subject')` en `nullableMorphs('causer')` deze composite indexes al aanmaakt (`subject` op `(subject_type, subject_id)`, `causer` op `(causer_type, causer_id)`). Geen aparte migratie nodig — geverifieerd via `information_schema.STATISTICS`. Acceptance criterium 12 daarmee al voldaan vóór WS-7 begon. Regression-guard: `tests/Feature/Database/ActivityLogIndexesTest.php` faalt wanneer een toekomstige refactor deze indexes verwijdert. + --- ## 4. Privacy / GDPR @@ -194,7 +213,7 @@ WS-7 is compleet wanneer: 9. Email-alerting geconfigureerd; getest met sample issue. 10. Retention-policy (90 dagen) toegepast. 11. Daily postgres-backup-script in place. -12. Activity_log indexes (addendum D-06) gemigreerd. +12. ~~Activity_log indexes (addendum D-06) gemigreerd.~~ ✓ — al voldaan door Spatie's `nullableMorphs` default in de originele activitylog migratie; zie §3.14 status-note. Regression-guard: `tests/Feature/Database/ActivityLogIndexesTest.php`. 13. Structured logging conventie geïmplementeerd; `X-Request-Id` round-trip getest. 14. SECURITY_AUDIT.md bijgewerkt.