`projectId` was nullable, and a null one made the row unreadable: every read
filters on that column, so an unattributed entry was written and then never
surfaced to anyone. The column is now NOT NULL, and `ActivityEventInput`
requires it.
The nullable column existed for one case, and that case turns out to be
fixable. `workflow-deleted` reaches a listener after the workflow row and its
`shared_workflow` rows have cascaded away, so the project cannot be looked up
at that point. But it can be captured before the delete: `workflow.service.ts`
already resolves the workflow, already calls `ownershipService`, and already
runs a `beforeWorkflowDeleted` hook whose stated purpose is capturing rows the
delete is about to cascade away. Enriching the event is upstream work, tracked
on the relay ticket.
Instance-level credentials are not an exception either. `instance_credential_-
assignment` maps a use slot to an ordinary credential that lives in a project;
what is instance-level is the assignment, not the credential.
Two test changes follow. The project-cascade test now writes to two projects
and asserts only the deleted one's entries go, since it can no longer lean on
an unattributed row surviving. And a new test confirms the column rejects an
entry with no project.
Raised in review: production executions are never linked to a user. They are
already out of the table, and their removal has a consequence the comments still
denied.
Every one of the eleven remaining events carries an acting user. So `userId` is
never null on insert; it goes null only when that user is deleted, which is what
the foreign key is for, and is why the column cannot be `NOT NULL`. The comment
said "for entries no user caused", which described a scheduled run and is now
unreachable.
Two more leftovers in the same file:
- The `activityResourceTypes` doc still explained how an execution entry points
at its workflow. It now carries the note that `resourceType` equals `category`
for every entry written today, and why the column stays anyway.
- The "deliberately not a foreign key" rationale sat above `resourceType` while
describing `resourceId`. Moved to the column it is about.
`deleteBeyondNewest(0)` deleted every row. n8n reads a zero retention count the
other way: `EXECUTIONS_DATA_PRUNE_MAX_COUNT` documents `0` as unlimited. A cap
wired from a config on that convention would therefore empty the table instead
of disabling the backstop. It now returns without deleting, and a real-database
test holds the table at three rows.
Four other review points:
- `deleteInBatches` took a batch predicate and a scope as separate arguments, so
a caller could pass a predicate that dropped its own scope. It now takes the
scope and an optional id bound, and adds the per-batch bound to that bound
instead of replacing it.
- `typeVersion` said it is bumped when the shape of `data` changes, but the only
write path could not set it. It is on the input now.
- Comments still described executions after they left the vocabulary: `failed`
and `succeeded` verbs, a scheduled run as the reason for a null `userId`, and a
run status in `data`. Two were Postgres column comments, so the schema docs are
regenerated.
- `resourceType` equals `category` for every entry written today. The comment now
says so, and says why the column stays: the two come apart for an entry about
one kind of thing that points at another, such as a source-control pull.
Review feedback on the migration: a row per workflow execution is too costly for
large instances. It is also redundant. `execution_entity` already records every
run and indexes `(workflowId, status, id)` for exactly the read a feed wants, so
an activity row per run duplicates an existing row at the same cardinality and
pays an insert on the execution hot path, on every worker. A reader queries that
table instead.
`category` therefore narrows to `workflow` and `credential` — the events with no
queryable home. A deleted workflow is the case that matters: `workflow_history`
cascades on delete, so nothing else records that the workflow existed or that it
was removed.
The trade-off is that the execution view now ends where `EXECUTIONS_DATA_MAX_AGE`
ends, 336 hours by default. That suits a recent-activity feed. A longer horizon
should copy the `insights` raw-to-period compaction, not a row per run.
Master landed `CreateAgentBackgroundJobTable1788191436461` above this migration.
The `migration-timestamp` rule requires every added migration to sort after all
others, so the file moves to 1788253087010.
The rebase also touched the two generated schema READMEs. Both are regenerated
rather than merged by hand.
`category` was documented as "Resource kind an entry is about", which is
`resourceType`'s job. Two columns whose docs claim the same role is how someone
eventually writes `execution` into `resourceType`.
They are different axes. `category` is what happened; `resourceType` is what the
entry points at. The two coincide for `workflow` and `credential` and diverge for
the rest — an execution and an eval run both point at a workflow, and only
`category` separates either from a save. `action` cannot stand in, because a real
run and an eval run share `succeeded` / `failed`.
The `migration-timestamp` code-health rule rejects a PR that adds more than one
migration: every added file must sort above every other, including its own
sibling, so the lower of the two always violates. The rule's own test named
"accepts a multi-migration PR" asserts exactly that in its body. Rebasing also
put both timestamps below `CreateTypeAvailabilityPolicyTables1787841960965`,
which landed on master meanwhile, so both needed renaming regardless.
Collapsing to one file forced a choice the ticket left open, and the resource
index is the part that goes. The ticket's own design note says it "arrives with
the read that needs it, not before", and nothing reads it here — so the index,
its decorator, `findByResource` and their tests are all deferred to the reader.
That leaves the two indexes each justified by a foreign key as well as a read.
The create-table migration is unchanged apart from its timestamp; it never
created the resource index.
Review follow-ups on the activity event table.
- `record` no longer accepts an `EntityManager`. Handing a driver handle to
callers is the leak `BaseRepository` and `TransactionRunner` exist to
prevent, and no caller wanted it: an entry describes something that already
happened, so it must not roll back with the operation that produced it.
- Drop `findById` and `findProjectIdForResource`. Both are reader work, both
arrived with no caller and no test.
- `projectIds` is required on a feed query. It is the access boundary, and an
omitted scope read every project on the instance. A comment saying "required"
is not an access check.
- A zero retention cap deletes everything instead of asking the driver for a
negative offset.
Two comments described mechanisms that do not hold. Pruning deletes the oldest
rows, so it never frees the highest SQLite rowid; only emptying the table does.
And a project deletion cannot be recorded against the project, because the
resource vocabulary has no entry for one.
Append-only record of what recently happened on an instance: an
`ActivityEvent` entity, the `activity_event` table, and a repository with
the single write path plus the two retention deletes.
Nothing reads or writes the table yet, so this ships and rolls back
independently of any behaviour change.
It lives in core `@n8n/db` rather than a module: a record of what changed
on the instance is not specific to one consumer, and scoping it to one
would make every later reader either cross a module boundary or build a
second log.
Three decisions worth keeping:
- `resourceId` has no foreign key. An entry has to outlive what it
describes — a workflow deletion is the entry most worth keeping, and a
cascade erases it exactly when it becomes interesting.
- No CHECK on `category` or `action`. A feed covers more kinds with every
feature, and each widening is another SQLite table recreation. The
single writer is a typed relay, so the compiler already rejects a value
outside the union.
- Two indexes on creation, `(projectId, id)` and `(userId, id)`, each
justified by a read and a foreign key. The per-resource index lands in
its own migration.
Index assertions read the index DDL back rather than going through
`getTable()`, which returns index columns alphabetically on Postgres and
so would confirm a column order it never checked.
