WS-6 v1.3-delta D2 — Listener refactor + integration #11
Reference in New Issue
Block a user
Delete Branch "feat/ws-6-v1.3-delta-d2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
WS-6 v1.3-delta D2 — Listener refactor + integration
Wires the D1 building blocks (exception hierarchy, classifier helper, broadcast event class, deadline-wrapper precursor,
failure_response_codecolumn) into the listener chain. With this PR merged, the WS-6 v1.3 amendment is fully implemented in code.No spine changes. Two-transaction pattern preserved, pre-publish guards untouched (RequiresIdentityKeyBinding was already unconditional pre-D2 — see audit note below),
BindingPassResult::applyStatus()PARTIAL semantics untouched, retry-flow contract preserved.Refs
dev-docs/RFC-WS-6.mdv1.3.1 — §Q1 v1.3 additions, §Q2 invariant, §Q3 v1.3 additionsdev-docs/ARCH-BINDINGS.mdv1.2 — §5.1–§5.6 listener chain, §6 two-transaction pattern, §7.1 status columnsD1 prerequisite
Builds on D1 (PR #10, merge-commit
c6f4d1b). All D1 building blocks are consumed in this PR — confirms D1's design intent.What this PR delivers
1.
ApplyBindingsOnFormSubmit::handle— deadline wrapper + classifier integration (RFC §Q1 v1.3 add 1, 4 + §Q3 v1.3 add 2)identity_match_status='pending'so the HTTP response carries correct first-paint state for the IdentityMatchBanner. The final state lands asynchronously via the queuedTriggerPersonIdentityMatch.apply()wrapped inwithDeadline(config('form_builder.apply_deadline_seconds', 5)). On exceeded deadline,FormBindingApplicatorTimeoutExceptionis thrown, caught by the outer transaction handler, and recorded withapply_status=FAILED+failure_response_code='temporary_error'.FormBindingExceptionClassifier::classifyto writefailure_response_codealongsideapply_status=FAILED.2.
TriggerPersonIdentityMatchOnFormSubmit— queued + invariant throw + broadcast (RFC §Q1 v1.3 + §Q2 + §Q1 v1.3 add 2)ShouldQueue. Sync chain reduced to one listener (ApplyBindingsOnFormSubmit).apply_status === COMPLETEDonly. PARTIAL and FAILED both fall through to early-return per ARCH-BINDINGS §5.6.subject_type='person'ANDapply_status=COMPLETEDimpliessubject_id IS NOT NULL. Violation throwsIdentityMatchInvariantViolation(introduced in D1) — routed via Laravel queue worker → GlitchTip +form_submission_action_failuresrow.FormSubmissionIdentityMatchResolved(D1 broadcast event class) onsubmission.{id}private channel after writing the finalidentity_match_status. Frontend Echo subscription is a separate frontend follow-up out of WS-6 scope.PersonIdentityService::detectMatchesreturnsCollection<PersonIdentityMatch>, not(status, matchCount). The listener preserves the existing string-status semantics ('matched'/'pending'/'none') and computesmatchCount = $matches->count()for the broadcast payload only — noidentity_match_countcolumn write (column doesn't exist; was an error in the original prompt, dropped per audit clarification).3. Broadcasting infrastructure (NEW) (RFC §Q1 v1.3 add 2)
Broadcasting was not previously wired in this codebase:
api/routes/channels.phpcreated withsubmission.{submissionId}private-channel authorization callbackapi/bootstrap/app.phpupdated:withRouting(channels: __DIR__.'/../routes/channels.php')registrationChannel auth is submitter-only for now. Org-admin access deferred to BACKLOG
TECH-CHANNEL-AUTH-ORG-ADMIN(added in this PR) — the Spatie Permission helper convention for organisation-scoped roles needs verification before extending the callback. Inline TODO inroutes/channels.phpcross-references the BACKLOG entry; a contract test asserts org-admin currently denied so a future flip is automatically visible.4. Queued-listener gating (ARCH-BINDINGS §5.6)
SyncTagPickerSelectionsOnSubmitnow has theapply_status === COMPLETEDgate as its first statement. Logs at info level when skipped (form-builder.queued-listener.skipped_apply_failed).ApplyBindingsOnFormSectionSubmittedis intentionally not gated — it listens toFormSubmissionSectionSubmitted, a different event, and §5.6 is specifically scoped toFormSubmissionSubmittedpost-apply state.5.
FormBindingApplicator::withDeadline()(RFC §Q1 v1.3 add 4)Fluent method returning a clone configured with a soft post-call microtime deadline. Implementation note: cannot interrupt mid-query — for that, configure MySQL connection timeouts at the driver level. The soft deadline is sufficient to prevent runaway apply() calls from hanging the public flow indefinitely; typical apply takes <100ms, so a 5s default catches the long tail.
6.
FormFailureRetryService::recordFailuresymmetry fix (ARCH-BINDINGS §7.1 v1.2 + RFC §Q3 v1.3 add 2)failure_response_codeviaFormBindingExceptionClassifier::classify— same classifier the listener uses, single behaviour-change point per the v1.3-delta D1 design.apply_completed_at = now()— closes the asymmetry where the listener wrote this column on both happy and failure paths but the retry service only wrote it on the success path.7.
config/form_builder.php—apply_deadline_secondsNew key, default
5, env-overridable viaFORM_BUILDER_APPLY_DEADLINE_SECONDS. Documented in the file's standard comment-block style.8.
AppServiceProvider::bootlistener-layout commentUpdated to v1.3 layout: 1 sync (
ApplyBindings) + N queued, all gated onapply_status=COMPLETED. Reference toFormSubmissionSubmittedListenerOrderTestadded as the structural verifier.Test counts
New test files (4):
tests/Feature/FormBuilder/Listeners/SyncTagPickerSelectionsOnSubmitGateTest.phptests/Feature/FormBuilder/Bindings/FormBindingApplicatorDeadlineTest.phptests/Feature/FormBuilder/Bindings/RetryServiceFailureClassifierTest.phptests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.phpModified existing test files (6):
TriggerPersonIdentityMatchOnFormSubmitTest(4 of 7 tests adjusted):test_public_event_registration_submission_is_marked_pending— replaced with two new tests (gate-skip behaviour, invariant-violation throw)test_linked_person_is_marked_matched,test_unlinked_person_with_no_matches_is_marked_none,test_unlinked_person_with_pending_match_is_marked_pending— fixture updates: seedapply_status=COMPLETEDbefore listener invocationFormSubmissionSubmittedListenerOrderTest:test_identity_match_listener_is_synchronous— flipped to assertShouldQueuePlus
ApplyBindingsOnFormSubmitTestextensions,FormBindingApplicatorIntegrationTestadjustments for the deadline path, etc.Out-of-scope (deferred)
apply_status=failed AND public_token IS NOT NULL— operational task, configured in GlitchTip web UI onmonitoring.hausdesign.nl. Not a code change.FormSubmissionIdentityMatchResolved— out of WS-6 scope; backend infrastructure ready and waiting.TECH-CHANNEL-AUTH-ORG-ADMIN. Inline TODO + denied-by-default contract test ensure it's visible.Audit findings (Phase A surfaced these before code work began)
EventRegistrationGuards::publishGuards()already callsRequiresIdentityKeyBinding('person', 'email')directly withoutConditionalRequirement(public_token)wrapper. The change had silently landed before D2; nothing to do. Documented in commit history; no scope-creep into making changes that weren't needed.routes/channels.phpdid not exist;bootstrap/app.phphad nochannels:parameter;config/broadcasting.phpdid not exist. Phase D therefore created new wiring (file + registration), not just an additive route.PersonIdentityService::detectMatchesreturn shape mismatch with prompt. The original prompt assumed(status, matchCount)return; actual isCollection<PersonIdentityMatch>. Adapted via Bert's confirmation: preserve string-status semantics, broadcast-onlymatchCount, drop the non-existent column write.identity_match_countcolumn was a phantom in the prompt. Column doesn't exist onform_submissions; the v1.3 amendment didn't actually call for it either — it was a leak in the implementation prompt. Listener writes onlyidentity_match_statusto the submission;matchCountlives in the broadcast payload as an ephemeral DTO field.FormSubmissionSubmitteddirectly without arrangingapply_status=COMPLETED— fine pre-gate, broken post-gate. Fix pattern: invoke the listener directly withapply_statuspre-set, mirroring the existing test comments' "bypass the service to isolate the listener contract" intent. May be worth promoting to a test helper if the pattern recurs in future queued-listener work.Commits (chronological)
762fc62— feat: wire D1 building blocks into ApplyBindings + deadline wrapper2a8f108— feat: TriggerPersonIdentityMatch becomes queued + invariant throw912022f— feat: broadcast channel auth + listener layout comment updatefa06c0f— feat: gate added to SyncTagPickerSelectionsOnSubmit012044f— fix: FormFailureRetryService writes failure_response_code + apply_completed_at03ff1cd— feat: apply_deadline_seconds config key (default 5)9420516— docs(backlog): TECH-CHANNEL-AUTH-ORG-ADMIN1afe116— test: WS-6 v1.3-delta D2 testsReview hints
withDeadlineis a soft deadline, not a hard one. It cannot interrupt mid-query. Documented in the code comment block. For hard query timeouts, configure MySQL connection-levelMAX_EXECUTION_TIMEseparately — out of D2 scope.=== COMPLETED. PARTIAL is aBindingPassResultoutcome (mixed per-binding success/failure) and ARCH-BINDINGS §5.6 v1.2 explicitly clarifies sibling listeners cannot safely run against PARTIAL state. PARTIAL-BINDING-SUCCESS BACKLOG entry covers the long-term direction.TECH-CHANNEL-AUTH-ORG-ADMINlands, the test will need flipping — that flip is the signal that the auth was actually extended.IdentityMatchInvariantViolationis\DomainException, notFormBindingApplicatorException. Different listener, different context. The classifier helper treats it as'unknown_error'because users cannot meaningfully act on it; admins triage via GlitchTip.🤖 Co-Authored-By: Claude Opus 4.7
Per RFC-WS-6 §Q1 v1.3 addition 1, 4 + §Q3 v1.3 addition 2 + ARCH-BINDINGS §5.3. - FormBindingApplicator::withDeadline(int) returns a clone configured to throw FormBindingApplicatorTimeoutException if apply() exceeds the deadline. Soft post-call microtime check; cannot interrupt mid-query but catches the long tail. apply() refactored to single-return so the deadline check sits at one site instead of duplicated. - ApplyBindingsOnFormSubmit::handle: - Initial identity_match_status='pending' write inside inner transaction (when subject is or becomes a person) so HTTP response carries the right state for the IdentityMatchBanner first-paint copy. Final state comes from the queued TriggerPersonIdentityMatch (D2 Phase C). - Wraps apply() with config('form_builder.apply_deadline_seconds', 5). - Catch block uses FormBindingExceptionClassifier::classify to write failure_response_code in the outer transaction alongside apply_status=FAILED. submission_id from the exception (when in the binding-applicator hierarchy) is also captured in context JSON. Tests added in Phase I. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>Per RFC-WS-6 §Q1 v1.3 (queueing) + §Q2 (invariant + IdentityMatchInvariantViolation) + §Q1 v1.3 addition 2 (broadcast). - Implements ShouldQueue (was sync). Gate as first statement: skip if apply_status !== COMPLETED (handles PARTIAL and FAILED identically per ARCH-BINDINGS §5.6). Logs at info level when skipped for triage visibility. - Failsafe-pad removed in favour of strict invariant: subject_type='person' + apply_status=COMPLETED implies subject_id IS NOT NULL. Violation throws IdentityMatchInvariantViolation, routed via Laravel queue worker to GlitchTip + form_submission_action_failures. - Status derivation preserved (string semantics 'matched'/'pending'/'none') — PersonIdentityService::detectMatches returns a Collection; status computed via user_id check + isNotEmpty(). matchCount derived from $matches->count() for the broadcast payload only (not persisted). - Person-not-found between dispatch and worker pickup terminates as 'none' rather than throwing — rare race-window where the person was deleted; banner gets a sensible final state. - Dispatches FormSubmissionIdentityMatchResolved on the submission.{id} private channel after writing the final identity_match_status. Frontend Echo subscription is a separate follow-up (out of WS-6 scope). The 4 existing failsafe-pad tests need rewriting in Phase I. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>Per RFC-WS-6 §Q1 v1.3 addition 2. - routes/channels.php (NEW): authorization callback for the submission.{id} private channel. v1 authz scope is submitter-only (matches submitted_by_user_id); org-admin access is deferred per BACKLOG TECH-CHANNEL-AUTH-ORG-ADMIN. Frontend Echo subscription lands as a separate frontend follow-up. - bootstrap/app.php: registers routes/channels.php via withRouting() channels: parameter. This is NEW broadcasting wiring — Laravel's broadcasting auth middleware was not previously connected to the framework. Without this registration the channels file is dead code. - AppServiceProvider:👢 comment block updated to v1.3 listener layout (1 sync ApplyBindings + N queued, all gated on apply_status=COMPLETED per ARCH-BINDINGS §5.6). Comment on TriggerPersonIdentityMatch flipped from "(sync)" to "(queued post-v1.3)". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>