crewli/dev-docs/RFC-WS-6.md

# RFC-WS-6 — FormBindingApplicator Pipeline Architecture

## 1. Status

- **State:** Authoritative for sessions 1, 2, 3 of WS-6
- **Frozen:** 2026-04-25
- **Owner:** Bert Hausmans
- **Origin:** Architectural session 2026-04-25 (Claude Chat) — 13 design decisions, 4 refinements, 3 observations
- **Related:**
  - `ARCH-FORM-BUILDER.md` §17 (form_field_bindings) and §31 (integration contracts)
  - `ARCH-CONSOLIDATION-2026-04.md` §6.1 (WS-5a binding table), §6.2 (WS-6 charter)
  - `ARCH-CONSOLIDATION-ADDENDUM-2026-04-24.md` Q1 (ULID exception retired), Q2 (denormalized organisation_id)
  - `SCHEMA.md` §3.5.12 (form_submissions, form_field_bindings)
  - `BACKLOG.md` items: FORM-BINDING-SNAPSHOT-MULTI, FORM-05, FORM-BUILDER-LIBRARY-AUDIT-LOG

## 2. Why this RFC exists

Charter §6.2 estimated WS-6 at 4-5 days. The architectural review on 2026-04-25 identified scope previously absent from the charter:

- Two-transaction failure-write pattern
- Separate `apply_status` column distinct from `identity_match_status`
- `form_submission_action_failures` table with retry/resolve/dismiss flows
- Dual-route admin UI (platform + organisation)
- Per-purpose `PublishGuard` framework with ~9 concrete guards
- Append-strategy collection-only restriction
- Section-level apply stub structure (feature-flagged)
- IDOR-class FK-chain policy

Revised estimate: **7-8.5 days across three Claude Code sessions**. The overrun is accepted — eerlijke scope, eerlijke inschatting. WS-6 lands as enterprise-grade fundament, not an MVP.

This RFC captures every decision so sessions 2 and 3 do not drift from the architecture established in session 1's planning phase.

## 3. Decisions

### Q1 — Listener ordering on `FormSubmissionSubmitted`

**Decision:** Two synchronous listeners in registration order, all other listeners queued and parallel.

```
SYNC chain (registered in EventServiceProvider in this order):
  1. ApplyBindingsOnFormSubmit         ← creates Person (when applicable), applies all bindings
  2. TriggerPersonIdentityMatchOnFormSubmit ← detects matches against the just-provisioned Person

QUEUED (no inter-ordering required):
  - SyncTagPickerSelectionsOnSubmit
  - CreateProvisionalShiftAssignmentsFromRegistration
  - FormWebhookDispatcher → DeliverFormWebhookJob
  - RegistrationConfirmation mailable (and other purpose-specific mailables)
```

Rationale: Laravel's listener-array order is the simple ordering primitive. `Subscriber` patterns and `$priority` flags add no value here. Sync vs. queued is the meaningful distinction — `identity_match_status` must be in DB before HTTP response serializes (per existing §31.1 UX choice for `TriggerPerson...`).

### Q2 — Refactor of `TriggerPersonIdentityMatchOnFormSubmit`

**Decision:** Trim the "no subject → pending" path to a logged warning failsafe. Do NOT merge with `ApplyBindingsOnFormSubmit`.

Post-WS-6, the listener should never see `subject_type === null` for `event_registration` submissions — `ApplyBindingsOnFormSubmit` resolves the Person via email-binding before this listener fires. The path becomes dead code, but rather than delete it we log a `Log::warning('form-builder.identity-match.no_person_subject_post_apply', [...])` and preserve the `'pending'` failsafe value. This catches misconfigured schemas (no email binding, no identity-key) and ApplyBindings silent failures with mechanical visibility.

The two listeners stay separate for testability, single-responsibility, and zero cost (two sync calls vs. one).

### Q3 — Strict-fail backend, log-and-swallow listener

**Decision:** `FormBindingApplicator` (service) throws strictly on invalid state; `ApplyBindingsOnFormSubmit` (listener) catches, records via `form_submission_action_failures`, swallows.

Service-layer throws on invalid `MergeStrategy`, missing required binding spec keys, deleted target columns, type-shape mismatches. Pre-publish guards (`PurposeGuardProvider::publishGuards()`) catch ~95% of these at config time. Runtime throws are reserved for "DB modified out from under us" rare cases.

Listener catches the throw, writes a row to `form_submission_action_failures` (in a separate transaction — see Q4), logs at error, swallows so sibling listeners (tag sync, shifts, webhooks, mail) keep running.

`SyncTagPickerSelectionsOnSubmit` is NOT folded into ApplyBindings. TAG_PICKER → `user_organisation_tags` is a pivot-table-with-source-discrimination operation, semantically distinct from a binding-target-attribute write. Document the deliberate parallel paths in ARCH-BINDINGS.md so future readers don't try to consolidate them.

### Q4 — Two-transaction atomicity pattern

**Decision:** ApplyBindings + Identity status update in one inner DB transaction. Failure-record write in a separate outer transaction. Event firing AFTER commit.

```php
try {
    DB::transaction(function () use ($submission) {
        $subject = $this->provisioner->provision($submission);
        $resolved = $this->resolveBindings($submission);
        $this->applyAll($subject, $resolved);
        $this->writeApplyStatus($submission, ApplyStatus::COMPLETED);
        $this->triggerIdentityMatch($submission);
    });
} catch (\Throwable $e) {
    DB::transaction(function () use ($submission, $e) {
        FormSubmissionActionFailure::create([
            'form_submission_id' => $submission->id,
            'listener_class'     => self::class,
            'failed_at'          => now(),
            'exception_class'    => $e::class,
            'exception_message'  => $e->getMessage(),
            'context'            => [...],
        ]);
        FormSubmission::query()
            ->whereKey($submission->id)
            ->update(['apply_status' => ApplyStatus::FAILED->value]);
    });
    Log::error('form-builder.apply.transaction_rolled_back', [
        'submission_id' => $submission->id,
        'exception'     => $e::class,
    ]);
}
```

Person provision uses `SELECT ... FOR UPDATE` on email lookup. Race conditions resolve to `firstOrCreate` semantics.

The inner transaction rollback survives the failure-record write because the failure row is in a different transaction. The second transaction is small (one insert + one update) — its failure path is Sentry-only, with explicit error log line for filterability.

`FormSubmissionSubmitted` event fires AFTER the inner transaction commits. This is non-negotiable: queued listeners (tag sync, shifts, webhooks, mail) must never see pre-commit state. Implementation uses explicit ordering in `FormSubmissionService::submit()` — service fires the event, not the Eloquent observer (per O2).

### Q5 — `form_submission_action_failures` complete in WS-6 (incl. admin UI)

**Decision:** Full table + write path + retry artisan command + dual-route admin UI (super_admin platform-wide + org_admin scoped) + retry/resolve/dismiss flows. All in WS-6 across sessions 2 (backend) and 3 (UI).

Schema:

```
form_submission_action_failures:
  id                    ulid pk
  form_submission_id    ulid fk → form_submissions, cascade delete
  listener_class        string(255)
  binding_id            ulid fk nullable → form_field_bindings, null on delete
  failed_at             timestamp
  exception_class       string(255)
  exception_message     text
  context               json
  retry_count           unsignedTinyInteger default 0
  resolved_at           timestamp nullable
  resolved_by_user_id   ulid fk nullable → users
  resolved_note         text nullable
  dismissed_at          timestamp nullable
  dismissed_by_user_id  ulid fk nullable → users
  dismissed_reason_type string(40) nullable    -- DismissalReasonType enum
  dismissed_reason_note string(500) nullable   -- required when type = 'other'
  created_at, updated_at
```

No soft delete. Retention follows the parent submission via cascade-delete.

**No `organisation_id` column** on this table — tenant scope flows via `form_submission_id → form_submissions.organisation_id`. See V3 below for policy implications.

Three actions in the admin UI:

- **Retry** — re-runs the applicator for this submission. Idempotent. Increments `retry_count`. On success: sets `resolved_at = now()`. On repeat failure: another row appended (history preserved).
- **Mark as resolved** — manual close, optional `resolved_note`. Used when an admin fixed the data via a different path.
- **Dismiss** — final close, requires `dismissed_reason_type` (enum), `dismissed_reason_note` required only when type = `OTHER`. Used when retry is impossible (schema deleted, target deleted).

Daily digest mailable is deferred to the notification framework (out of scope WS-6 — depends on infrastructure not yet in place).

### Q6 — Snapshot is truth, not live schema

**Decision:** `FormBindingApplicator` reads bindings from `form_submissions.schema_snapshot.fields[*].bindings`, not from the live `form_field_bindings` table.

Webhook payload, GDPR export, and audit replay all use `schema_snapshot`. Bindings follow the same pattern. The live table is consulted only for pre-publish validation by `FormSchemaService`.

This guarantees that a retry-from-failures-table executed a week after the original submission applies the bindings as they were configured at submit time, not as they may have been edited since. Reproducibility for audit.

### Q7 — Conflict resolution: candidate set + trust precedence

**Decision:** Candidate set = bindings whose source form_field has a row in `form_values` for this submission, regardless of whether the value is null. Sort by `trust_level DESC`, tie-break by `form_field.sort_order ASC`. Empty winner writes null when `merge_strategy` allows it.

The candidate distinction matters: a row in `form_values` with `value = null` is an **explicit clear** by the user (multi-step form, edit-resubmit). Absence of a row means the field was not submitted (conditional logic skipped it, validation rejected it, etc.). The binary "row exists" is the gate; value content is decided afterwards.

Per-strategy null-winner matrix:

| `merge_strategy` | Winner value = null | Behaviour |
|---|---|---|
| `overwrite` | null | Write null to target (intent: clear) |
| `append` | null | No-op (nothing to append; target unchanged) |
| `replace` | null | No-op when target is already non-null; no-op when target is null |
| `first_write_wins` | null | Write null when target is null (claim the slot); skip when target has any value |

`append` is restricted to collection-typed targets per V1 — the matrix entry above describes the no-op case for completeness.

**Write-path invariant test required (session 2):** for every form_field that should be visible after conditional-logic evaluation at submit time, a `form_values` row exists after submit, even with null value. Otherwise "explicit null" is indistinguishable from "skipped by logic" and Q7 collapses.

### Q8 — Single identity-key per target_entity

**Decision:** At most one binding per `(target_entity)` may have `is_identity_key = true`, enforced by the `MaxOneIdentityKeyPerTargetEntity` publish guard. `PersonProvisioner::provisionFromBindings` uses `Person::firstOrCreate(['email' => $value, 'organisation_id' => $orgId], $otherBindings)`. Race-safe via DB-driver `firstOrCreate` semantics.

Composite identity (email OR (first_name + last_name + DOB)) is **out of scope for v1.0**. Backlog item `FORM-BINDING-COMPOSITE-IDENTITY` tracks the future work. Identity-matching against existing user accounts (via `person_identity_matches`) is a separate flow, not in scope here — that's `PersonIdentityService` running after provision.

### Q9 — Apply uniform across all 7 purposes; provisioning per `PurposeDefinition`

**Decision:** `FormBindingApplicator::apply()` is purpose-agnostic. Subject resolution lives on `PurposeDefinition::resolveOrProvisionSubject(FormSubmission, PersonProvisioner): ?Model`.

```
event_registration       → PersonProvisioner::provisionByEmailBinding (may create)
artist_advance           → ArtistResolver::fromPortalToken (existing context, throws if absent)
supplier_intake          → CompanyResolver::fromProductionRequest
post_event_evaluation    → PersonResolver::fromAuth
incident_report          → PersonResolver::fromAuth OR null (anonymous-allowed)
signature_contract       → UserResolver::fromAuth
user_profile             → UserResolver::fromAuth
```

Test coverage: 4 cases per purpose minimum (happy-path, missing required binding, conflict-resolution, anonymous-when-applicable) → 28 + ~20 purpose-agnostic = ~48 pipeline tests in session 2.

### Q10 — Section-level submit: stub now, activate later

**Decision:** Build the listener-class structure now; gate the runtime via `config('form_builder.section_apply_enabled', false)`; activate when ARTIST_ADVANCE feature work lands.

```php
class ApplyBindingsOnFormSectionSubmitted implements ShouldQueue
{
    public function handle(FormSubmissionSectionSubmitted $event): void
    {
        if (!config('form_builder.section_apply_enabled', false)) {
            return;
        }
        $this->applicator->apply($event->submission, sectionId: $event->sectionId);
    }
}
```

`FormBindingApplicator::apply()` accepts an optional `sectionId` parameter. Null = all bindings. Set = only bindings whose source form_field's section_id matches. Listener registered in `EventServiceProvider` from session 2.

**Publish guards land NOW for section-aware schemas, regardless of feature flag.** A schema with `section_level_submit = true` and an `is_identity_key` binding in section 3 is structurally unsafe; it must not pass publish even if the runtime listener is gated off. The `IdentityKeyBindingsOnlyInFirstSection` guard wires into every PurposeGuardProvider as a universal guard from session 1 (no-op for non-section schemas).

Feature flag has explicit removal trigger documented in `config/form_builder/section_apply.php` comment block: "Enable when ARTIST_ADVANCE feature work begins. At enablement: write section-scoped tests, activate stub listener registration, remove this flag and the early-return guard. Tracking: BACKLOG.md → ARTIST-ADV-SECTION-APPLY."

A minimal wiring test asserts the stub listener is registered and forwards correctly to the applicator signature — no business logic, just keeping the path alive.

### Q11 — Library-binding propagation: copy at instantiation, no runtime cascade

**Decision:** When a `form_field` is created from a `FormFieldLibrary` entry with bindings, the library-bindings are **copied** to `form_field_bindings` rows with `owner_type = 'form_field'`. Subsequent updates to the library-bindings do NOT propagate to existing field instances. Future "library re-sync" admin action lives on the BACKLOG.

This matches the WS-5d decision for `form_field_options` and is consistent with template-source semantics. Predictable for versioning. Documented explicitly in ARCH-BINDINGS.md: "the library is a template-source, not a live link."

### Q12 — Activity log: hierarchical pass + per-binding entries

**Decision:** One `form_submission.bindings_pass_completed` activity per applicator invocation, plus one `form_submission.binding_applied` activity per binding (linked via `properties.parent_activity_id`).

```
activity 1: form_submission.bindings_pass_completed
  subject: form_submission Z
  properties: {
    binding_count: 12,
    succeeded: 11,
    failed: 1,
    person_provisioned: true
  }

activity 2..13: form_submission.binding_applied
  subject: person X (or whichever target entity)
  properties: {
    parent_activity_id: <activity 1 id>,
    target_entity: 'person',
    target_attribute: 'email',
    old_value: null,
    new_value: 'jan@example.nl',
    trust_level: 80,
    merge_strategy: 'overwrite',
    source_form_field_id: ...,
    source_submission_id: Z
  }
```

Spatie/laravel-activitylog supports arbitrary properties; `parent_activity_id` is just a properties field. UI renders this hierarchically as a collapse: pass-level visible, per-binding expandable. Failed bindings get their own activity entry too (`properties.error_class`/`error_message`), in addition to their `form_submission_action_failures` row.

Two sources of truth for failures (activity_log + action_failures) is intentional: activity_log is the human-readable timeline, action_failures is the machine-replayable workflow.

### Q13 — Pre-publish validation: `PurposeGuardProvider` framework

**Decision:** Pre-publish rules live on a parallel `PurposeGuardProvider` interface (NOT on `PurposeDefinition`, which stays an immutable value object). Each purpose's concrete provider returns a `list<PublishGuard>`. `FormSchemaService::publish()` walks the list, runs every guard, collects all violations, and throws a `PublishGuardViolationException` with the full list (not first-fail).

```php
interface PurposeGuardProvider
{
    /** @return list<PublishGuard> */
    public function publishGuards(): array;
}
```

Nine concrete guards in `App\FormBuilder\Publishing\`:

1. `RequiresIdentityKeyBinding(entity, attribute)` — flag-check (binding existence enforced separately by `assertRequiredBindingsPresent`)
2. `MaxOneIdentityKeyPerTargetEntity` — universal
3. `RequiresFieldType(type, minCount)`
4. `SchemaHasLinkedEvent`
5. `TagCategoriesConfiguredOnAllPickers`
6. `IdentityKeyBindingsOnlyInFirstSection` — universal (no-op for non-section schemas)
7. `AppendStrategyRequiresCollectionTarget` — universal (paired with V1)
8. `NoAmbiguousTrustLevels` — universal
9. `ConditionalRequirement(predicate, subGuard)` — higher-order composer

Universal guards (2, 6, 7, 8) wire into every purpose. Purpose-specific guards per the Task 8 table.

The `PurposeGuardProvider` is wired into `purposes.php` via a new `guards_class` key per purpose. `PurposeRegistry::guardProviderFor(string $slug)` instantiates and caches.

Open-closed: a new purpose adds its own `PurposeDefinition` config entry plus a new `PurposeGuardProvider` class; `FormSchemaService::publish()` does not change.

## 4. Refinements

### V1 — Append strategy is collection-only

**Decision:** `merge_strategy = APPEND` is valid only when the target attribute resolves to `BindingTargetType::COLLECTION` per `BindingTypeRegistry`. Pre-publish guard `AppendStrategyRequiresCollectionTarget` enforces this.

Rationale: append on scalars (string concatenation, comma-separated lists) requires a fingerprint mechanism to detect duplicate-append on retry. Embedding fingerprints in domain data is an architectural smell. Collection types with SET semantics (JSON array with deduplication, pivot relations) make retry naturally idempotent. This restriction eliminates the entire problem class.

The `BindingTypeRegistry` is the single source of truth for target shape — NOT name-suffix matching (e.g. attributes ending in `_tags` or `_skills`). Name-suffix matching is convention-not-contract and would silently misclassify any attribute that doesn't follow the convention or accidentally matches it (e.g. `user_tags_count` would falsely qualify as collection).

### V2 — `DismissalReasonType` enum + resolve action separated

**Decision:** Six enum values (`schema_deleted`, `target_entity_deleted`, `binding_removed`, `duplicate_submission`, `data_quality_issue`, `other`). Free-text `dismissed_reason_note` required only for `other`. **Resolve is a separate action** with its own `resolved_note` (no enum), distinct from dismiss.

Three admin actions on a failure:

- **Retry** — replay the applicator
- **Mark as resolved** — manual close (succeeded via another path)
- **Dismiss** — final close (will not be replayed); requires reason enum

Dismiss-without-enum was rejected because "diversen-syndroom" makes failure analytics impossible after six months. `manually_resolved` as a dismissal reason was rejected because resolve and dismiss are semantically different workflows.

### V3 — IDOR-class FK-chain policy

**Decision:** `FormSubmissionActionFailurePolicy` resolves tenant via `failure.submission.organisation_id`, NOT via route binding. Cross-tenant access returns `false` from the policy; controllers translate to 404 (not 403, which would confirm resource existence and enable enumeration).

Security tests in this class are explicitly named as IDOR-class so they are findable in audit. Pattern follows ARCH §22.9 `FormResourceSecurityTest`.

### V4 — Concurrent-test via PHPUnit state-injection

**Decision:** PHPUnit-based concurrent test with state-injection (insert conflicting Person row inside `lockForUpdate` window, assert `firstOrCreate` recovery). NOT an Artisan command + manual TEST_SCENARIO.md step.

A handmatige test is geen test. State-injection in PHPUnit tests the recovery path under the actual conflict condition — which is what matters. Real wall-clock load testing belongs in a separate "Load-test foundation" workstream against staging (BACKLOG: LOAD-TEST-FOUNDATION).

## 5. Observations

### O1 — `apply_status` default for legacy rows = NULL

Existing seed-data and pre-WS-6 staging submissions have no apply_status. Migration adds the column WITHOUT a default. Admin UI's "open work" filter explicitly excludes NULL rows so legacy submissions don't appear as "pending applicator work that will never run." Document in migration file comment block.

### O2 — Event firing AFTER `DB::afterCommit()` (or explicit post-commit dispatch)

`FormSubmissionSubmitted` must fire after the inner transaction commits, not before. Implementation: `FormSubmissionService::submit()` runs the transaction, then explicitly calls `event(new FormSubmissionSubmitted($submission->refresh()))`. Do NOT rely on Eloquent observers firing the event from within the transaction — that pre-commit fire would let queued listeners enqueue with state that may never commit.

### O3 — RFC-WS-6.md as session-handover anchor

This document. Sessions 2 and 3 reference RFC sections by number rather than re-establishing context. Drift between chat decisions and code implementation is prevented by committing the RFC before any session-1 code lands.

## 6. Out of scope (explicit non-goals for v1)

- **Composite identity-key resolution** (multi-attribute matching) → BACKLOG: `FORM-BINDING-COMPOSITE-IDENTITY`
- **Cross-event submission deduplication** (one Person, multiple events with separate registrations) → handled by existing identity-match flow, not a binding concern
- **Library-binding runtime cascade** (updates propagate to instantiated fields) → BACKLOG: `FORM-LIBRARY-RESYNC`
- **Append strategy on scalar targets** → architecturally rejected (V1)
- **Active section-level apply** → stub structure only; activated when ARTIST_ADVANCE feature work begins (BACKLOG: `ARTIST-ADV-SECTION-APPLY`)
- **Daily failure digest mailable** → depends on notification framework not yet built
- **Wall-clock concurrent load testing** → BACKLOG: `LOAD-TEST-FOUNDATION` (separate workstream)

## 7. Sessions split

| Session | Scope | Estimate |
|---|---|---|
| **1 — Foundation** | Schema migrations (apply_status, action_failures), enums (ApplyStatus, DismissalReasonType, BindingTargetType), MergeStrategy enum methods, value objects (ResolvedBinding, BindingApplicationResult, BindingPassResult), BindingTypeRegistry + config, Models, FormSubmissionActionFailurePolicy, PublishGuard framework + 9 concrete guards, PurposeGuardProvider interface + 7 concrete providers, FormSchemaService publish integration, ARCH-BINDINGS.md skeleton, this RFC | 2-2.5 days |
| **2 — Pipeline** | FormBindingApplicator service, PersonProvisioner, multi-purpose subject resolvers, listener chain (sync ApplyBindings + sync TriggerPersonIdentityMatch + queued siblings), two-transaction pattern, ApplyBindings stub for section-submit, retry/resolve/dismiss artisan commands + controllers + Form Requests + Resources, all backend tests (~100-120 new), ARCH-BINDINGS.md sections 6-9 filled | 3-4 days |
| **3 — Admin UI** | Vuexy admin UI on `/platform/form-failures` (super_admin) and `/orgs/{org}/form-failures` (org_admin), retry + resolve + dismiss flows, IDOR-class API security tests, ARCH-BINDINGS.md final, ARCH-OBSERVABILITY.md initial draft | 1.5-2 days |

**Total: 7-8.5 days.** Charter §6.2 had 4-5 days; the overrun is explicit and accepted (see §2 of this RFC).

## 8. Test coverage targets

| Session | New tests | Cumulative backend |
|---|---|---|
| Pre-WS-6 baseline | — | 1208 |
| Session 1 | ~90-110 | ~1300-1320 |
| Session 2 | ~110 | ~1410-1430 |
| Session 3 | ~35 | ~1445-1465 |

## 9. Open follow-ups (post-WS-6)

| BACKLOG item | Trigger |
|---|---|
| `FORM-BINDING-COMPOSITE-IDENTITY` | When a purpose requires multi-attribute identity resolution |
| `FORM-LIBRARY-RESYNC` | When organisations report friction updating library-derived fields |
| `ARTIST-ADV-SECTION-APPLY` | When ARTIST_ADVANCE feature work begins (post-S5) |
| `LOAD-TEST-FOUNDATION` | Pre-release hardening, separate workstream |
| `FORM-BINDING-SNAPSHOT-MULTI` | When patterns require multi-binding-per-field snapshot shape |
| Daily failure digest | When notification framework lands |

## 10. Document history

- 2026-04-25 — v1.0 — Initial RFC, frozen at start of WS-6 session 1.