solcom · solcom-review · in-session · 3 Achsen (Canvas-Achse inaktiv)

solcom-review: mp/P2503-90 — SOLCOM Vacancy Application (Drupal-native Bewerber-Flow)

mp/P2503-90develop Reviewer: Claude Opus 4.7 (1M) Datum: 2026-05-28 Story Hub: F-100-bewerber-flow.html
standards: fail  |  canvas: skipped  |  spec: fail Worst: HIGH — 5 PHP-Fatals / Test-Suite 17/17 errors vor Fix; danach 4/17 failures wg. Webform-_meta-Serialization-Bug.

Scope & Fixed-Point

Drei-Wege-Diff develop...mp/P2503-90, zwei Commits, +2868 Insertions, 19 neue Files (1 Modul solcom_vacancy_application). Adressiert Story F-100 / P2503-90 mit Sub-Tasks P2503-184 (Webform+Sub-Route+Queue+SMTP), P2503-186 (Webhook-Subscribe + Webhook-Receiver + ACK), P2503-187 (DLQ-Admin-UI), P2503-188 (Smoke-Test-Drush). Architektur-Pfad: C+ Email-Inbox-Import + Webhook-ACK (per Story-Hub-Status REFINED, 2026-05-25).

Canvas-Achse skipped — keine Touches in config/sync/canvas.*.yml oder web/themes/custom/**/*.component.yml; ein Webform-Config-File config/install/webform.webform.vacancy_application.yml, kein SDC-Verschnitt.

Standards-Achse FAIL

S-1 — HARD Fatal: redeclared readonly $loggerFactory + missing FileSystemInterface import (PersonioWebhookController)

PersonioWebhookController definiert private readonly LoggerChannelFactoryInterface $loggerFactory via Constructor-Promotion — das kollidiert mit der nicht-readonly Property ControllerBase::$loggerFactory der Parent-Klasse, PHP 8.4 fatal: "Cannot redeclare non-readonly property ... as readonly". Zusätzlich fehlt das use Drupal\Core\File\FileSystemInterface;-Statement; der Constructor-Parameter typed gegen \Drupal\solcom_vacancy_application\Controller\FileSystemInterface (relative namespace lookup), Class-not-found. Beide Fatals zusammen verhindern das Laden der Klasse — ALL 17 Kernel-Tests vor Fix errorten mit 0 Assertions.

Evidence (Test-Output vor Fix):

PHP Fatal error: Cannot redeclare non-readonly property
Drupal\Core\Controller\ControllerBase::$loggerFactory as readonly
Drupal\solcom_vacancy_application\Controller\PersonioWebhookController::$loggerFactory
in .../PersonioWebhookController.php on line 25
...
Tests: 17, Assertions: 0, Errors: 17.

FIXED in this review$loggerFactory$logChannelFactory umbenannt, use Drupal\Core\File\FileSystemInterface; ergänzt. PHPStan-Level-1 nach Fix: "[OK] No errors". Tests laden jetzt; 13/17 PASS (Tests & Assertions: 17, 118).

S-2 — HARD Fatal: missing FileSystemInterface import (DlqAdminController)

Identisches Pattern wie S-1 in DlqAdminControllerFileSystemInterface ohne use. Wird beim ersten Request auf /admin/karriere/dlq mit Class-not-found fatalen. FIXED: Import ergänzt.

S-3 — HARD Fatal: missing RequestStack import (PersonioWebhookCommands)

PersonioWebhookCommands::__construct typed gegen RequestStack ohne use Symfony\Component\HttpFoundation\RequestStack; — Class-not-found beim Drush-Command-Discovery. AK-12 (drush solcom:personio:webhook-subscribe) ist damit nicht ausführbar. FIXED: Import ergänzt.

S-4 — HARD Fatal: missing QueueWorkerManagerInterface import (PersonioSmokeTestCommands)

PersonioSmokeTestCommands::__construct typed gegen QueueWorkerManagerInterface ohne entsprechendes use. Class-not-found beim Drush-Command-Discovery. AK-16 (drush solcom:personio:smoke-test) ist damit nicht ausführbar. FIXED: Import ergänzt.

S-5 — HARD QueueWorker Backoff ist nicht implementiert — AK-9 strukturell unerfüllt

ApplicationQueueWorker::handleSendFailure() wirft beim Retry throw new \RuntimeException(...). Drupal Core Queue API behandelt eine geworfene Exception als re-claim ohne Delay — die Konstante BACKOFF_SECONDS = [60, 300, 900, 3600, 21600] wird zwar ins Submission-Meta geloggt, der Worker wird aber sofort beim nächsten queue:run erneut aufgerufen. Es gibt keinen next_attempt_after-Setter im Code, obwohl die Check-Logik in processItem() ihn liest. Mechanismus ist broken-by-design:

NICHT auto-fixbar — erfordert refactoring des Workers (DelayedRequeueException + Drush-Queue-Driver-Check, ob lokale Datenbank-Queue Delay tatsächlich respektiert; SuspendQueueException ist nicht äquivalent).

S-6 — HARD Webform-Meta-Storage-Bug: _meta wird als String "Array" persistiert

Das Webform vacancy_application deklariert _meta als '#type': hidden mit '#default_value': ''. Hidden Webform-Fields casten Werte zur String-Repräsentation beim Save. Code ruft $submission->setElementData('_meta', ['status' => 'X', ...]) — das Array wird beim Reload als String "Array" zurückgeliefert (PHP-Array-to-String-Cast).

Evidence: Tests DlqAdminControllerTest::testRequeueResetsMeta und testResolveMarksManuallyResolved erwarten $meta als Array, bekommen aber den String "Array":

Failed asserting that 'Array' is of type array.

Konsequenzen über Story-Surface hinweg:

NICHT auto-fixbar — erfordert Architektur-Decision: (a) _meta in einer separaten key_value-Tabelle (z. B. state:solcom_personio.submission_meta.<sid>) speichern, oder (b) ein Custom Field-Type mit Serialized-Storage statt Hidden-Field, oder (c) JSON-encode/decode beim Setter/Getter (Workaround, aber leakt Type-Sicherheit).

S-7 — HARD CSRF-Vulnerabilität: DLQ Re-Queue/Resolve sind GET-Routes

solcom_vacancy_application.dlq_requeue und .dlq_resolve sind methods: [GET], ändern aber Submission-State (Re-Queue erzeugt Queue-Items, Resolve markiert "manuell erledigt"). Keine CSRF-Token-Validierung. Ein eingeloggter HR-User, der einen präparierten Link in einer Mail anklickt, triggert die State-Change ungewollt.

Fix-Empfehlung: methods: [POST] + _csrf_token: 'TRUE' in den Routing-Defs, oder Confirm-Form via ConfirmFormBase. NICHT auto-fixbar (verändert API-Surface der Action-Links im Listing — muss in Tests + UI Template mitziehen).

S-8 — HARD Webform-Submission-Storage-Resolution im Container ohne Cast

PersonioWebhookController::create() und DlqAdminController::create() resolven entity_type.manager->getStorage('webform_submission'), aber EntityStorageInterface ist die Rückgabe-Type-Annotation — nicht WebformSubmissionStorageInterface. Der PHPDoc-Cast (@var WebformSubmissionStorageInterface) trickst PHPStan, aber zur Laufzeit hängt es davon ab, dass webform_submission's Storage-Definition tatsächlich auf die Webform-Interface zeigt. Das tut sie (per webform.module), aber die Implementation ist fragil gegen Renames in Webform-Contrib. Niedrige Priorität (Webform >=6 stable), aber verdient einen instanceof-Guard oder einen Fallback.

JUDGE NICHT auto-fixbar — Stil-Frage, nicht funktional kaputt.

S-9 — JUDGE Webhook-Signature-Verify ist optional (no-secret = no-check)

PersonioWebhookController::receive() überspringt die Signatur-Prüfung, wenn solcom_personio.webhook_secret in State leer ist (Branch $secret === ''). Das ist als Dev-Ergonomie gemeint, aber öffnet in Production ein Fenster, falls der Webhook-Subscribe-Drush-Command nie gelaufen ist oder die State-Var manuell gelöscht wurde — ein unauthenticated POST-Body kann Submissions markieren und PII purgen. Logging-Warning ist da, blockt aber nicht.

Fix-Empfehlung: in settings.php per Konvention bei $config['environment'] === 'production' harte Pflicht erzwingen, oder ein Permission-Check-Layer für die Receiver-Route (z. B. IP-Allowlist der Personio-Webhook-Ranges).

S-10 — JUDGE Personio-Webhook-Subscribe-Payload-Schema

PersonioWebhookCommands::webhookSubscribe() POSTet {event_type, url, secret} nach /v2/webhooks/subscriptions. Die Story-Hub Sektion "Spike-Result" erwähnt, dass die Webhook-API POST/PATCH/DELETE erlaubt, aber nicht das genaue Body-Schema. Der echte Personio-API-Schema (per OpenAPI webhooks-service-v2.yaml) verlangt typischerweise { subscriptions: [{event_name, callback_url}] } oder {name, callback_url, event_names: [...]} — nicht event_type. Smoke-Test gegen Live-Personio würde 400 zurückgeben.

Empfehlung: Schema gegen Live-Personio-Endpoint probe bevor Production-Deploy (Story-Hub Open Question #3 ist noch unbeantwortet). Bis dahin nicht-deployable.

S-11 — JUDGE Performance: loadByProperties ohne Limit lädt alle Submissions

PersonioWebhookController::matchSubmission() ruft $submissionStorage->loadByProperties(['webform_id' => 'vacancy_application']) — bei n=10000 Lifetime-Submissions lädt das alle ins RAM und iteriert sequenziell durch getElementData('_meta'). Skaliert nicht über MVP-Volumen.

Empfehlung: per entity.query() mit Filter auf hidden tracking_hash-Feld, sobald _meta auf strukturierten Storage umgebaut ist (siehe S-6).

S-12 — JUDGE PII-Purge incomplete: nur Element-Data, nicht Submission-Lifetime

acknowledgeSubmission() setzt PII-Element-Daten auf NULL und löscht private://applications/<uuid>/. Aber: Webform speichert auch eine submitted_data-Serialization im webform_submission_data-Table und ggf. Logs unter webform_submission_log. Ein zusätzliches $submission->resetData() oder explizite Delete von Sub-Data-Records ist nicht da. AK-14 ("PII-Felder gepurged") wird formal erfüllt, aber Submission ist nicht GDPR-clean.

Empfehlung: Statt PII-Felder zu NULLen, kompletten $submission->delete(); den Personio-Application-ID separat in State oder einer kleinen Custom-Table cachen für Backlink-Audits.

Canvas-Achse SKIPPED

Keine Touches in config/sync/canvas.*.yml, keine *.component.yml-Änderungen, keine SDC-Templates. tool_api_validate_canvas_tree nicht aufgerufen.

Spec-Achse FAIL

Pre-Fix: alle 17 AKs unerfüllt durch S-1/S-2/S-3/S-4 Fatals. Post-Fix-Status pro AK:

AK-#StatusBegründung
AK-1untestedApply-CTA auf Vacancy-Detail-Seite — Anbindung in solcom_vacancy-Modul nicht im Diff sichtbar; gehört aber zu P2503-90 Implementation-Scope. Manuelle Verifikation erforderlich.
AK-2partialSub-Route + Webform + Template existieren; byte_theme-Branding nicht im Diff geprüft (Template enthält nur generische CSS-Klassen vacancy-apply-page__*; kein Lime-Accent, kein Montserrat-Mount). Browser-Verify ausstehend.
AK-3untestedField-Inventur gegen karriere.solcom.de-Referenz ist Implementation-Phase-Step ohne Test-Coverage. Webform hat 10 Felder + privacy_accepted + 3 hidden; agent-browser-Inventur ausstehend.
AK-4passWebform-Config: cv required + 10MB + pdf doc docx; cover_letter optional; additional_attachment optional & multiple. ✓
AK-5passprivacy_accepted required + Wording mit <a href="/datenschutz">. ✓
AK-6partialApplicationQueueHandler erzeugt Queue-Item + Redirect; postSave() only on insert. Aber: Files-Storage-Pfad ist private://applications/ root (Webform-Config-Defaultwert), nicht private://applications/<submission-uuid>/ wie AK fordert. Module-Code erwartet aber den UUID-Subpfad (Purge in acknowledgeSubmission(), DLQ-Listing-Pfad-Anzeige). Drift zwischen Webform-Config und Code.
AK-7passHoneypot + time_restriction in third_party_settings.honeypot. ✓
AK-8partialQueueWorker + MailBuilder bauen Subject mit trace:<hash>-Format korrekt (Tests verifizieren das). Attachments-Handling über solcom_vacancy_application_mail() hook angelegt — aber: Drupal Symfony Mailer braucht params.files in einer spezifischen Struktur (filepath/filename/filemime), die Hook-Implementation schreibt es da hin; Verifikation gegen real-mail-test-collector ausstehend.
AK-9FAILSiehe S-5 — Backoff strukturell nicht implementiert (plain RuntimeException statt DelayedRequeueException).
AK-10partialDLQ-Mail-Path + private://-Persistenz im Code; aber Submission-Status-Setzen auf dlq_persistent scheitert an S-6 (Meta-Storage-Bug) → Submission bleibt auf retry_pending.
AK-11FAILDLQ-Listing ist via S-6 permanent leer (status-filter matcht nie). Permission-Check existiert und ist korrekt. CSRF-Lücke S-7 zusätzlich.
AK-12FAILDrush-Command-Discovery scheiterte vor Fix (S-3). Post-Fix funktional, aber Payload-Schema (S-10) gegen Live-Personio ungetestet.
AK-13partialSignature-Verify Tests OK; Match-Logik leidet unter S-6 (sent_pending_ack-Status nie persistiert als Array).
AK-14partialPII-Purge-Logik vorhanden, aber unvollständig (S-12); ACK-Status-Setzen scheitert an S-6.
AK-15partialTests: 17 angelegt — 13 PASS, 4 FAIL post-fix. Failures sind alle Root-Cause S-6 (_meta-Serialization). 118 Assertions exekutiert.
AK-16partialDrush-Command-Discovery scheiterte vor Fix (S-4). Post-Fix vorhanden; Live-Read-back ungetestet (Personio v2 query-Parameter candidate.email existiert per Spike-Result, aber URL-Encoding fehlt).
AK-17n/aAVV ist HR/Legal-Sub-Task (P2503-90.8), nicht im Code-Scope.
AK-18passState-Key solcom_personio.inbox_email ist im QueueWorker via $state->get() referenziert, nicht in config/sync/. ✓

Acceptance: FAIL — 3 AKs hart fail (9, 11, 12), 7 AKs partial/untested, 5 AKs pass, 1 n/a. Inconclusive zählt per ADR-0028-Konvention als fail.

Applied Deterministic Fixes (this review)

Vier missing-import-Fatals + ein readonly-Konflikt deterministisch fixbar; in der Working-Tree applied, Test-Suite läuft jetzt (vorher 17/17 errors, jetzt 13/17 pass). Commit-Empfehlung: fix(P2503-90): review polish (PHPStan imports, readonly conflict) — aber wegen S-5/S-6/S-7 als BLOCKING-Findings nicht als "Ready for Merge".

Verification: ddev exec vendor/bin/phpstan analyse web/modules/custom/solcom_vacancy_application/ --level=1 → "[OK] No errors". PersonioMailBuilderTest 4/4 PASS, 36 Assertions. Gesamt 13/17 PASS, 118 Assertions.

Outstanding Findings (NOT auto-fixed)

  1. S-5 (HIGH): Backoff via DelayedRequeueException($delay) implementieren — AK-9 blocker.
  2. S-6 (HIGH): _meta-Storage-Architektur entscheiden (KeyValue vs. Custom-Field vs. JSON-Encode-Wrapper) — AK-11/13/14 blocker.
  3. S-7 (HIGH): DLQ-Action-Routes auf POST + CSRF-Token oder ConfirmForm umstellen.
  4. S-10 (JUDGE): Webhook-Subscribe-Payload-Schema gegen Live-Personio probe; ggf. {subscriptions: [{event_name, callback_url}]}-Form umstellen.
  5. S-12 (JUDGE): PII-Purge zu vollem Submission-Delete erweitern (GDPR-Komplett-Cleanup).
  6. S-9, S-11 (JUDGE): Webhook-Secret in Prod erzwingen + Match-Performance per entity_query verbessern (kann mit S-6-Fix kombiniert werden).
  7. AK-2 / AK-6 / AK-8 / AK-16: Browser-Verify-Pflicht ausstehend — sobald Modul installierbar ist (post S-6-Fix oder Workaround), QA-Plan-Click-Pfade abarbeiten.

Recommended Next Steps

  1. Commit der deterministisch gefixten Imports (fix(P2503-90): review polish (imports, readonly conflict)) — bringt das Modul zumindest zum Laden, ist Voraussetzung für jede weitere Verifikation.
  2. Architektur-Entscheidung S-6 (Meta-Storage) — sollte als ADR-Entwurf (kein Komponenten-Topologie-Decision, aber persistenz-Design) geführt werden, weil es Konvergenz mit ähnlichen Patterns im Repo verdient.
  3. Refactor des QueueWorker (S-5) als atomarer Folge-Commit; danach Re-Run der Test-Suite mit Mock-Mail-System.
  4. Spike: Personio-Webhook-Subscribe-Payload-Schema (S-10) gegen Live-API mit papi--Credentials probe (analog zur Spike-Sequenz im Story-Hub).
  5. Browser-Verify (AK-2, AK-6, AK-8, AK-16) erst nach S-6/S-5/S-10 sinnvoll.