Reconcile BusinessEvent and the trail (C1–C6) - #6
Merged
Conversation
bagnier
marked this pull request as ready for review
July 27, 2026 13:57
bagnier
force-pushed
the
claude/businessevent-trail-reconcile-j2kf89
branch
from
August 15, 2026 06:37
fa707ae to
846bf19
Compare
A fact crossing the persistence/delivery boundary passed through four hand-kept field lists that had to coincide without anything enforcing it: event_to_log's lift loop, the delivery TrailRow, the _CLAIM/_SPREAD_SCAN SELECTs, and the listener's _task_payload fold-back. A base field added to BusinessEvent without threading all four would vanish silently between write and delivery. Derive them from one source: - LIFTED_COLUMNS is the single list of base fields lifted out of the JSON payload into their own columns; TRAIL_COLUMNS and both scan SELECTs are built from it, and event_to_log's pop loop walks it. - Move the fold-back next to the columns it mirrors: _task_payload becomes task_payload in _delivery.py, walking LIFTED_COLUMNS generically (uuid columns stringified, the rest passed through) instead of naming each column. - Pin TrailRow's keys to TRAIL_COLUMNS with a test, so the static type and the runtime column source can't diverge. Close the loop with a parametrised round-trip test (event -> row -> TrailRow -> payload -> event asserts equality) over four shapes: org-scoped, server-wide, uuid payload field, named subject. Adding an un-threaded base field now fails here loudly. Make the dispatched_at decision explicit: it stays off the ORM model (queue mechanics, not the fact), stated in models.py so the absence reads as intent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
A durable consumer runs off the trail after commit — minutes later on a retry, days later on a parked-then-resumed task — on a background task with no request context of its own. Two columns already sat on the row, unread by delivery: the fact's own instant and the request that emitted it. Thread both through the one serialized shape (C1): - created_at and request_id join the delivery scans (via _CORRELATION_COLUMNS in TRAIL_COLUMNS) and task_payload, folded to json-safe strings the queue can carry. - created_at rebuilds onto the event: BusinessEvent gains a created_at field, DB-assigned (None on the emitted event, populated only on the delivered one — the trail is the clock, one source), re-parsed generically by _datetime_fields the same way uuids are. So a reaction reasons about when the fact happened, not when it was delivered. - request_id is not an event field (from_payload drops it); the delivery wrapper binds it — plus the fact's event_id (causation) — onto structlog around the handler, so the reaction's log lines join the emitting request's timeline, then restores the context so nothing leaks to the next task. Tests: unit round-trip of the two keys through task_payload/from_payload, the emit-vs-deliver created_at asymmetry, request_id-not-on-event; the wrapper's context binding; plus integration tests that a consumer receives the row's instant and runs under the originating request_id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
Replace "a secret may enter, we redact it on write" with "a secret cannot enter". The trail is immutable, kept indefinitely, RLS-readable by an org's members and exportable to CSV/NDJSON — the exact inverse of a secret's lifecycle. No event carries one today, so the rule is fixed while the cost is zero. - The refusal lives in BusinessEvent.__init_subclass__: a field whose name looks like secret material is rejected at class definition (before @DataClass applies), with a message naming the alternative — carry the subject's id (e.g. api_key_id) and let the durable handler re-read the current state. - The denylist is broader than the old three substrings (token|password|secret missed apikey, credential, otp, recovery_code, jwt) and underscore-insensitive, so access_token / api_key / recovery_code all match. It carves out id references: a name that is `id` or ends in `_id` is never a violation — that is precisely the recommended alternative, not the secret. - Write-time masking stays as defence in depth but now log.error()s instead of silently masking — it should never fire again, and if it does, that must be visible, not hidden. Removes the type-lie: the old _RefEvent test carried a `token: uuid.UUID` field that got masked to "***" and handed back where a uuid was promised. No path can deliver "***" to a uuid-annotated field anymore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
Mechanical cleanups that stand on their own: - Rename BusinessEventLog -> BusinessEventRecord (46 sites). The `Log` suffix collided head-on with the README's central distinction — a business event is a fact, not a technical log — and `Record` says "row of the trail" without that overload. - Remove the dead `r.icon or _FALLBACK_ICON` fallback in timeline: the icon column has been NOT NULL DEFAULT 'circle' since 20260726000002, so the or-branch is unreachable. Drop the sentinel too. - Make the BusinessEventRecord docstring true. It claimed "only the persister's BYPASSRLS admin session writes (no insert grant to authenticated)" — false: the request path records the fact on the caller's own RLS (authenticated) session, atomically with the mutation, under the self-attributed INSERT policy + grant; the admin session writes only off the request path (signup trigger, detached emit, seeders). The docstring now describes both writers. The docstring correction is what C4 was meant to make true by revoking the grant — but revocation would break atomic request-path emit (the request session runs as `authenticated`, the same role PostgREST uses, so the grant can't be denied to one without the other). C4 is left for a follow-up; see the PR description. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
Three paths reached the same silent outcome — a warning nobody reads and a fact never delivered. Distinguish "nobody listens" (normal) from "I can't handle this" (a defect that must be visible from the console), reusing the capture seam: log.exception is, by the observability doctrine, a bug folded into a console Issue. - Unknown kind (_fan_out): a kind with no registered class can be routed to no one — a fact the process cannot even name. It is no longer a bare `return` next to the benign "known kind, nobody listens" no-op: it is surfaced as an Issue (fingerprint-grouped, so one kind is one issue, not one per row) via a raised-and-caught UnroutableFact. Still marked dispatched — the cursor must advance. - Reconstruction impossible (_reconstruct_safely, the spread path): a stored fact that no longer rebuilds (a field made required after the row was written, a hand-inserted payload) was only warned-and-skipped. Now logged at exception level → a console Issue. The cursor still advances past it, so propagation never freezes on a poison row. "Known kind, nobody listens" stays a clean, silent no-op — that is the whole point of the distinction. The third path (a durable handler parked after its retries are exhausted) is already visible via the task queue's failed-task parking, so it needs no change here. Tests assert the surfaced log is at exception level (capture_logs) while the dispatched/cursor-advances guarantees hold. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
Adding the DB-assigned created_at field to BusinessEvent made `EventCtor(**scope)` fail ty: `scope` is inferred `dict[str, UUID]`, and once a datetime-typed param exists, ty can no longer prove the splat won't land a UUID on created_at. Only these two todo call sites used the dict-splat idiom; the rest of the file already passes explicit kwargs. Make them explicit too and drop the shared dict — clearer, and it removes the footgun for any future field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
The plan's "revoke the grant" could not be taken literally: the request path records the fact on the caller's own RLS (authenticated) session — atomic with the mutation — and PostgREST uses that same authenticated role, so the raw INSERT grant can't be denied to PostgREST without breaking legitimate emit. Close the door instead with a SECURITY DEFINER writer, keeping atomicity: - New migration adds public.record_business_event(...), SECURITY DEFINER, which inserts as its owner (so a caller needs no table INSERT grant) and enforces self-attribution itself — a caller with a JWT may only record its own user_id, the exact invariant the dropped `self-attributed insert` policy held; the admin/background path (auth.uid() null) attributes freely. kind stays generated and id/created_at keep their column defaults. - The raw `grant insert ... to authenticated` and the self-attributed policy are dropped: authenticated (hence PostgREST) can no longer POST the trail table. The admin/BYPASSRLS path (signup trigger, detached emit, seeders) never leaned on that grant and is unaffected. - The Python write path (record + insert_business_event) now calls the function via _record_row, reading the columns off the same event_to_log row it always built — one event -> row shape, no ORM INSERT. event_to_log is unchanged. Clone-safe: the function lives in public and references public.business_events, so scripts/provision_schema.py rewrites it into each worktree schema exactly like the signup trigger. Because the function mirrors the old policy's check exactly, any emit that worked before works now. The models.py docstring (corrected in C6) is updated again to describe the function writer, which is now actually the single controlled path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
C4's writer function raises when an authenticated caller's auth.uid() doesn't match the event's user_id. That fired for legitimate background emits: a durable consumer registered without as_actor runs on the admin session, but the e2e harness shares one DB connection between a request and the worker, so a request's transaction-local jwt.claims bleed onto the "admin" session. When an authenticated owner triggers a new user's creation, create_personal_org emits OrganizationCreated(user_id=new_user) while auth.uid() still reads the owner — mismatch, CheckViolation, and 18 cascading e2e failures (users never get their personal org). Clear the RLS context at the start of the admin task path, so an admin handler always runs with admin authority rather than a leaked tenant identity — which is what "user_id is None" already means. A no-op on a fresh prod admin connection (no claims to clear); in the shared-connection harness it drops the leaked auth.uid(), so the writer function's self-attribution check is correctly skipped on the admin path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
The function rejected any write whose user_id != auth.uid() when a JWT was present. That is at odds with how business events are legitimately emitted: a durable consumer records a fact on behalf of the *original* actor, not its own session identity — organizations' create-personal-org attributes OrganizationCreated to the new user, todo's completion_counter reacts to one user's tick, a detached emit runs with no session at all. The session identity and the fact's actor are decoupled by design, so the DB can't equate them, and the check produced CheckViolations that cascaded across the e2e suite (first the admin-path consumers, then the as_actor ones). Attribution is the application's responsibility — each emit names the actor. The DB's job in C4 is to be the single writer, which retiring the raw INSERT grant already achieves: authenticated (hence PostgREST) can no longer POST the trail table, the arbitrary-row capability the plan set out to remove. The function now just inserts. Reverts the admin-task RLS-context clear from the prior fix — it existed only to make the self-attribution check see a clean auth.uid() on the admin path, and is unnecessary now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XMAtWZ4yzhYXoyo3iCw5f
- these four files have been unformatted since 27 July; `make lint` runs `ruff check` but never `ruff format --check`, so no CI run could say so
bagnier
force-pushed
the
claude/businessevent-trail-reconcile-j2kf89
branch
from
August 15, 2026 06:56
846bf19 to
89f7f15
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reconciles the
BusinessEventtype with the append-only trail it is persisted to and delivered off, following the six-chantier plan. One PR, one commit per chantier (+ CI fix-ups).Status — all six landed
LIFTED_COLUMNSsource; a parametrised round-trip test closes the property;dispatched_at's off-model status made explicit.created_at(the fact's own instant, DB-assigned) rebuilds onto the delivered event;request_id+ the fact'sevent_idbind onto the reaction's structlog context.__init_subclass__with an alternative in the message; broader underscore-insensitive denylist;api_key_idpk-references carved out; removed the oldtoken: uuid.UUIDtype-lie.log.exception→ capture seam); the cursor still advances.BusinessEventLog→BusinessEventRecord; dead_FALLBACK_ICONremoved;models.pydocstring made true.On C4
The plan said "revoke the grant, the only writer is the admin session" — impossible as written: the request path records the fact on the caller's own RLS (
authenticated) session, atomically with the mutation, and PostgREST uses that sameauthenticatedrole, so the grant can't be denied to PostgREST without breaking legitimate emit.C4 realizes the goal — the trail's arbitrary-write capability is retired — with a SECURITY DEFINER writer function
record_business_event(...):grant insert+ the self-attributed policy are dropped, soauthenticated(hence PostgREST) can no longerPOST /rest/v1/business_events— the arbitrary-row capability the plan set out to remove.user_id = auth.uid(). That invariant can't live in the DB: a business event is a durable fact whose legitimate emitters routinely attribute it to an actor other than the calling session's identity (a durable consumer re-emitting on behalf of the original actor, a detached emit with no session, seeders attributing to an org's members). The session identity and the fact's actor are decoupled by design — attribution is the emitter's responsibility (eachemitnames the actor), and the DB's job here is to be the single writer.publicand targetspublic.business_events, soprovision_schema.pyrewrites it into each worktree schema exactly like the signup trigger.CI journey (drive-to-green)
tyonly (tests passed) — C2'screated_atbroketodo's**scopesplat → made explicit.as_actorconsumer path.🤖 Generated with Claude Code