refactor(core): Narrow the activity event repository to its write path (no-changelog)

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.

docs/generated/postgres-schema/public.activity_event.md

@@ -9,7 +9,7 @@
 | createdAt | timestamp(3) with time zone | CURRENT_TIMESTAMP(3) | false |  |  |  |
 | data | json |  | true |  |  | Minimal detail that makes an entry meaningful unexpanded (a run status and failing node, a save node delta). Size-capped on write; no user ids |
 | id | integer |  | false |  |  |  |
-| projectId | varchar(36) |  | true |  | [public.project](public.project.md) | Scopes the entry to one project. NULL for instance-level entries — including a project deletion, which would otherwise cascade away the record of itself |
+| projectId | varchar(36) |  | true |  | [public.project](public.project.md) | Scopes the entry to one project. NULL for entries about the instance rather than one project |
 | resourceId | varchar(36) |  | true |  |  | Id of the resource, for fetching the full record. No FK: entries outlive it |
 | resourceName | text |  | true |  |  | Name at the time of the entry, denormalised so a row reads without a join and survives the resource being deleted. Truncated on write |
 | resourceType | varchar(32) |  | true |  |  | What `resourceId` points at; see ActivityResourceType in @n8n/db. NULL when an entry is about the instance rather than a resource |

packages/@n8n/db/src/entities/activity-event.ts

@@ -35,8 +35,9 @@ export class ActivityEvent extends WithCreatedAt {
 	/**
 	 * Autoincrement int, not the usual nanoid: the feed orders by id, pages on it as a cursor, and
 	 * records the high-water mark of what a thread was already shown. On SQLite this is a rowid
-	 * alias, so ids are reused once the top rows are deleted, which pruning does — never compare a
-	 * stored cursor against a feed that has been pruned beneath it.
+	 * alias: pruning deletes the oldest rows and so never frees the highest id, but emptying the
+	 * table does, and ids then restart. A stored cursor is only meaningful against a feed that has
+	 * held at least one row throughout.
 	 */
 	@PrimaryGeneratedColumn()
 	id: number;
@@ -62,6 +63,11 @@ export class ActivityEvent extends WithCreatedAt {
 	@Column({ type: 'uuid', nullable: true })
 	userId: string | null;
 
+	/**
+	 * Null for entries about the instance rather than one project. A project deletion cascades its
+	 * entries away; recording that deletion needs a resource vocabulary that covers projects, which
+	 * is why no writer emits one yet.
+	 */
 	@Column({ type: 'varchar', length: 36, nullable: true })
 	projectId: string | null;
 

packages/@n8n/db/src/migrations/common/1787741452169-CreateActivityEventTable.ts

@@ -38,8 +38,8 @@ export class CreateActivityEventTable1787741452169 implements ReversibleMigratio
 				column('projectId')
 					.varchar(36)
 					.comment(
-						'Scopes the entry to one project. NULL for instance-level entries — including ' +
-							'a project deletion, which would otherwise cascade away the record of itself',
+						'Scopes the entry to one project. NULL for entries about the instance rather ' +
+							'than one project',
 					),
 				column('resourceType')
 					.varchar(32)
@@ -69,9 +69,9 @@ export class CreateActivityEventTable1787741452169 implements ReversibleMigratio
 				columnName: 'id',
 				onDelete: 'SET NULL',
 			})
-			// CASCADE: a deleted project's history is unreachable by every read, which are all
-			// project-scoped. The one entry that must survive a deletion is written at instance
-			// level instead, with the project id in `resourceId`.
+			// CASCADE: a deleted project's history is unreachable by every read, since all of them
+			// are project-scoped. Recording the deletion itself would need an entry outside the
+			// project, which no writer emits today.
 			.withForeignKey('projectId', {
 				tableName: PROJECT_TABLE,
 				columnName: 'id',

packages/@n8n/db/src/repositories/__tests__/activity-event.repository.test.ts

@@ -98,10 +98,15 @@ describe('ActivityEventRepository', () => {
 		it('combines both id bounds instead of letting one overwrite the other', async () => {
 			entityManager.find.mockResolvedValueOnce([]);
 
-			await repository.findFeed({ limit: 10, afterId: 5, beforeId: 40 });
+			await repository.findFeed({
+				projectIds: ['project1'],
+				limit: 10,
+				afterId: 5,
+				beforeId: 40,
+			});
 
 			expect(entityManager.find).toHaveBeenCalledWith(ActivityEvent, {
-				where: { id: And(MoreThan(5), LessThan(40)) },
+				where: { projectId: In(['project1']), id: And(MoreThan(5), LessThan(40)) },
 				order: { id: 'DESC' },
 				take: 10,
 			});
@@ -147,6 +152,16 @@ describe('ActivityEventRepository', () => {
 			expect(deleted).toBe(7);
 		});
 
+		it('deletes everything when the cap is zero, rather than asking for a negative offset', async () => {
+			entityManager.delete.mockResolvedValueOnce({ affected: 3, raw: [] });
+
+			const deleted = await repository.deleteBeyondNewest(0);
+
+			expect(entityManager.find).not.toHaveBeenCalled();
+			expect(entityManager.delete).toHaveBeenCalledWith(ActivityEvent, {});
+			expect(deleted).toBe(3);
+		});
+
 		it('does nothing when the table holds fewer entries than the cap', async () => {
 			entityManager.find.mockResolvedValueOnce([]);
 

packages/@n8n/db/src/repositories/activity-event.repository.ts

@@ -1,6 +1,6 @@
 import { Service } from '@n8n/di';
-import { And, DataSource, In, LessThan, MoreThan, Not, IsNull, Repository } from '@n8n/typeorm';
-import type { EntityManager, FindOptionsWhere } from '@n8n/typeorm';
+import { And, DataSource, In, LessThan, MoreThan, Repository } from '@n8n/typeorm';
+import type { FindOptionsWhere } from '@n8n/typeorm';
 import type { IDataObject } from 'n8n-workflow';
 
 import { ActivityEvent } from '../entities';
@@ -20,11 +20,11 @@ export type ActivityEventInput = Pick<ActivityEvent, 'category' | 'action'> &
 export type ActivityFeedQuery = {
 	limit: number;
 	/**
-	 * The projects whose entries the caller may see. Required for any agent-facing read: without
-	 * it a read spans every project on the instance, including ones the user cannot open.
-	 * Entries with no project are excluded, since there is no way to prove they are in scope.
+	 * The projects whose entries the caller may see. Required, not optional: an omitted scope
+	 * would read every project on the instance, including ones the user cannot open. Entries
+	 * with no project are excluded, since there is no way to prove they are in scope.
 	 */
-	projectIds?: string[];
+	projectIds: string[];
 	userId?: string;
 	resourceId?: string;
 	category?: ActivityEvent['category'];
@@ -42,12 +42,11 @@ export class ActivityEventRepository extends Repository<ActivityEvent> {
 
 	/**
 	 * The only write path, so the row-size guarantees hold whatever the caller passes.
-	 * `trx` lets a caller bind the entry to its own unit of work; the event relay does not,
-	 * because an entry describes something that already happened.
+	 * Deliberately not transaction-aware: an entry describes something that already happened, so
+	 * it must not be rolled back with the operation that produced it.
 	 */
-	async record(input: ActivityEventInput, trx?: EntityManager): Promise<void> {
-		const repository = trx ? trx.getRepository(ActivityEvent) : this;
-		await repository.insert({
+	async record(input: ActivityEventInput): Promise<void> {
+		await this.insert({
 			category: input.category,
 			action: input.action,
 			userId: input.userId ?? null,
@@ -66,13 +65,11 @@ export class ActivityEventRepository extends Repository<ActivityEvent> {
 	 * the current user's own actions apart from everyone else's.
 	 */
 	async findFeed(query: ActivityFeedQuery): Promise<ActivityEvent[]> {
-		const where: FindOptionsWhere<ActivityEvent> = {};
-		if (query.projectIds !== undefined) {
-			// An empty allowance means nothing is visible, not everything — `In([])` would match no
-			// row on Postgres but is worth being explicit about rather than relying on it.
-			if (query.projectIds.length === 0) return [];
-			where.projectId = In(query.projectIds);
-		}
+		// An empty allowance means nothing is visible, not everything — `In([])` would match no
+		// row on Postgres but is worth being explicit about rather than relying on it.
+		if (query.projectIds.length === 0) return [];
+
+		const where: FindOptionsWhere<ActivityEvent> = { projectId: In(query.projectIds) };
 		if (query.userId !== undefined) where.userId = query.userId;
 		if (query.resourceId !== undefined) where.resourceId = query.resourceId;
 		if (query.category !== undefined) where.category = query.category;
@@ -105,32 +102,6 @@ export class ActivityEventRepository extends Repository<ActivityEvent> {
 		});
 	}
 
-	/** One entry by id, for expanding it. Callers check its project against their own scope. */
-	async findById(id: number): Promise<ActivityEvent | null> {
-		return await this.findOne({ where: { id } });
-	}
-
-	/**
-	 * The project a resource was last seen in, from the log's own earlier entries.
-	 *
-	 * This is the only way to scope an entry recorded after the resource is gone: by the time
-	 * `workflow-deleted` fires, the sharing rows that would name its project are deleted too.
-	 * Without it such entries have no project, and an agent-facing read has to drop them — which
-	 * would silently lose deletions, the entries most worth surfacing.
-	 */
-	async findProjectIdForResource(
-		resourceType: NonNullable<ActivityEvent['resourceType']>,
-		resourceId: string,
-	): Promise<string | undefined> {
-		const [latest] = await this.find({
-			where: { resourceType, resourceId, projectId: Not(IsNull()) },
-			order: { id: 'DESC' },
-			take: 1,
-			select: { projectId: true },
-		});
-		return latest?.projectId ?? undefined;
-	}
-
 	/** Retention by age. Returns how many entries went, so a caller can log a sweep worth noticing. */
 	async deleteOlderThan(cutoff: Date): Promise<number> {
 		const { affected } = await this.delete({ createdAt: LessThan(cutoff) });
@@ -143,6 +114,9 @@ export class ActivityEventRepository extends Repository<ActivityEvent> {
 	 * deletes below it, rather than counting rows twice.
 	 */
 	async deleteBeyondNewest(keep: number): Promise<number> {
+		// `skip` is an offset, so a zero cap would ask for -1. Keeping nothing means deleting all.
+		if (keep <= 0) return (await this.delete({})).affected ?? 0;
+
 		const [oldestKept] = await this.find({
 			order: { id: 'DESC' },
 			skip: keep - 1,