Skip to content

Commit 8e4a230

Browse files
fix(plugin-audit): a user's own sys_user_preference write no longer reaches their bell as a sys_activity row (#22996)
Fixes #22961 Clause-②: no A user's own write to their own `sys_user_preference` row (the console's `ui.recent`, `ui.favorites` and palette recents) no longer adds a `sys_activity` row, so it no longer reaches the user's bell as `Updated User Preference "ui.recent"`. The `sys_audit_log` row for the same write is unchanged. ## What the bell entries are, measured The card marked its mechanism "to confirm". Measured on `origin/main` `a2240c4dc`: - **They are `sys_activity` rows, not inbox messages.** A throwaway probe (not committed) used the real `ObjectKernel`, `ObjectQLPlugin`, `AuditPlugin`, SQLite and the shipped `SysUserPreference` object. It created and then updated a user's own `ui.recent` row, acting as that user. Each write produced one `sys_audit_log` row and one `sys_activity` row. The update's summary was `Updated User Preference "ui.recent"` (the card's text), with `actor_id` set to the user. The objectui bell's Activity tab lists the newest `sys_activity` rows the caller may read. Its second line is `timeAgo · object_name`, which matches the card's `· sys_user_preference` subtitle; notification rows show no object name. - **Nothing turns a record write into a `sys_inbox_message`.** `service-messaging` registers no data hook. The only callers of `emit()` are approvals, comment @mentions, the automation `notify` node and the HTTP nodes. Subscription expansion (`sys_notification_subscription`) is not wired, and its object docblock says so. So the inbox half of the card's acceptance item 3 already holds on `main`, and the activity mirror is the channel to change. - **Other writers of a preference row.** Members are limited to their own rows by RLS (`sys_user_preference_self`, `user_id == current_user.id`). An organization admin's wildcard `modifyAllRecords` passes that RLS, and Setup lists the object (`nav_user_preferences`), so an admin can edit another user's row. No server-side code writes the object. These writers keep today's rows. - **Which objects are in the class.** The console keeps all of its per-user UI state on `sys_user_preference`: objectui's `ConsoleShell` binds its `ui.favorites`, `ui.recent` and `ui.flow.palette.recents` adapters to it. No other per-user UI-state object exists. No spec key means "per-user UI state". The existing `enable.activities: false` mutes every writer, which is wider than this rule. ## The change In `packages/plugins/plugin-audit/src/audit-writers.ts`: - Beside `SKIP_OBJECTS`, a closed, module-private map, `PER_USER_UI_STATE_OWNER_FIELD` (`sys_user_preference` maps to `user_id`). Its comment writes the rule down: what reaches a user's bell is another actor's act or something the user needs, never the user's own UI state. - In `writeAudit`, the writer keeps the ledger row and skips the activity row when both of these hold: - the object is in the map; - the user the activity row would name (`userId`, the `actor_id` value) is the owner on every side of the row the write carries. It also skips the actor-name read, which only feeds the activity row. This is the same seam as the existing `enable.activities` opt-out. - Not touched: `SKIP_OBJECTS`; `AUDIT_EXCLUDED_OBJECTS`, which the hook registration and `read-audit.ts` read; the package entry. `dist/index.d.ts` names neither new symbol (0 hits; the control `installAuditWriters` has 3). The `sys_audit_log` row a preference write produces is unchanged. ## Pins (`audit-writers.test.ts`) - The owner's own create, update and delete of a `sys_user_preference` row each produce exactly one `sys_audit_log` row (`action`, `user_id` and `record_id` asserted) and zero `sys_activity` rows. - CONTROL: the same user's update of an ordinary audited object keeps its activity row. - CONTROL: on a user's preference row, each of these keeps its activity row, with the expected `actor_id`: - a write by another user (an admin); - a system write with no user; - the owner moving the row to another user. Ablations ran through `scripts/ablation-replace.mjs` in wrap mode. For each one the tool showed the anchor count going 1 to 0 on disk and the blob changing. It then proved the restore: blob equal to `HEAD` and an empty `git diff HEAD`. They ran on `2632f1ee5`, whose plugin-audit sources are byte-identical to the head: 1. **Skip removed** (`mirrorsActivity = activitiesEnabled`): `Tests 3 failed | 4 passed`. The three own-write pins fail with `AssertionError: expected [ { object: 'sys_activity', …(1) } ] to have a length of +0 but got 1`. 2. **Blanket mute** (owner check removed): `Tests 3 failed | 4 passed`. The three other-writer controls fail with `AssertionError: expected [] to have a length of 1 but got +0`. 3. **Ledger row dropped** (`sys_user_preference` added to `SKIP_OBJECTS`): `Tests 6 failed | 1 passed`. The own-write pins fail at `audit-writers.test.ts:727`, the `sys_audit_log` length assertion. The first attempts at ablations 2 and 3 were no-ops. Their replacement text contained the anchor, so `ablation-replace` saw the anchor count stay at 1, refused before running the tests, and restored the file. Both were re-run with anchors the replacement does not contain. The readings above are from the re-runs. The card's acceptance item 1 (a browser walk) was not done. The pins carry it at the writer seam that produces the rows. ## Verification - `@objectstack/plugin-audit` unit suite, at `2632f1ee5` (later commits change only the changeset): `Test Files 47 passed (47)`, `Tests 750 passed (750)`. - `pnpm --filter @objectstack/plugin-audit typecheck` (`tsc`, the scripts project and `check:test-typecheck` over `tsconfig.test.json`): exit 0. - Dependency closure (`turbo run build --filter=@objectstack/plugin-audit^...`, 15 tasks) and the package's own build (`check-dts-emitted`: 1/1 declaration file present): exit 0. - Derived gates: `node scripts/pm/dispatch-gates.mjs --commands` over this branch (3 paths, 67 families), every command run and reconciled with `--ran`. - At `03373429f`: `Run reconciliation — 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN`, with every recorded exit code 0. - A first pass at `2632f1ee5` recorded `check:dual-build-cjs-loads` and `check:i18n` at exit 3 (PREREQUISITE NOT MET), because other packages had no `dist/`. Later in that pass, `check:type-check-debt` built the whole workspace through `turbo run typecheck`. - On the head pass both measured green: 107 CommonJS entries across 66 packages load, and 9 packages' translation bundles are in sync. - Lint, scoped to the touched files, with three pieces of evidence: - eslint's own config claims both files (`--print-config`: 6 and 5 rules); it claims nothing for the changeset `.md`. - `eslint --no-inline-config --format json` over the two files: 2 files, 0 errors, 0 warnings, at `03373429f`. - `eslint.config.mjs` never enables type-aware linting (no `parserOptions.project`, no typed rules, stated at its line 327), so this diff cannot change the result for any file it does not touch. - `origin/main` was merged before opening the PR. It was still `a2240c4dc` (remote tip read with `ls-remote`), so the merge was a no-op. ## Acceptance notes - Activity rows written before this change are not deleted. They leave the feed when the 14-day `sys_activity` retention (`lifecycle.retention.maxAge: '14d'`) removes them. The changeset says so. - For a skipped activity row, the writer still composes the summary and resolves the locale, exactly as it does today for `enable.activities: false`. That work is wasted but has no observable effect. Not filed. - The bell's Activity tab shows `sys_activity.object_name` verbatim (objectui `InboxPopover.tsx`, filled by `activityItemType.ts`). Every other activity row therefore still shows a machine name such as `crm_lead` rather than the object's label. This is reported to the seat as a candidate in objectui. It is not in this PR. --- _Generated by [Claude Code](https://claude.ai/code/session_01CeRCvvvjxbxu2iYJAc5Cce)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: objectstack-fleet[bot] <332303061+objectstack-fleet[bot]@users.noreply.github.com>
1 parent 929c8b1 commit 8e4a230

3 files changed

Lines changed: 184 additions & 2 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
---
4+
5+
fix(plugin-audit): a user's own write to their own `sys_user_preference` row no longer adds a `sys_activity` row. The `sys_audit_log` row is written as before
6+
7+
Clause-②: no
8+
9+
The console saves the user's "recently visited" list (`ui.recent`), favourites and palette recents as `sys_user_preference` rows as they navigate. The audit writer mirrored each of those writes into `sys_activity`, and the bell's Activity tab lists the newest activity rows the caller may read. So every navigation came back to the user as `Updated User Preference "ui.recent"`, and a fresh user's bell showed nothing else.
10+
11+
- **What changed.** For a per-user UI-state object, the writer skips the activity row when the acting user is the row's owner (`user_id`). The list of such objects is closed and declared beside the writer's exclusion list. Today it holds one object, `sys_user_preference`.
12+
- **What stays.** The `sys_audit_log` row for the same write is written exactly as before. Another actor's write to a user's preference row keeps its activity row: an organization administrator editing it from Setup, a system write with no user, or a write that gives the row a different owner. Writes to every other object are unchanged.
13+
- **Existing rows.** Activity rows written before this release are not deleted. They leave the feed when the `sys_activity` retention window (14 days) removes them.
14+
- **Nothing to migrate.** No configuration is needed.

‎packages/plugins/plugin-audit/src/audit-writers.test.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,106 @@ describe('audit writers — enable.activities opt-out gate (#2707)', () => {
686686
});
687687
});
688688

689+
/**
690+
* [#22961] A user's own write to their own per-user UI-state row writes the
691+
* ledger row and no activity row. Every other writer keeps today's rows.
692+
*
693+
* The rows below are the console's "recently visited" write as the real engine
694+
* hands it to this hook: the session subject is the owner, and both `result`
695+
* and the bound `previous` carry the full row with its `user_id`.
696+
*/
697+
describe('audit writers — a user\'s own per-user UI-state write is not mirrored (#22961)', () => {
698+
const SCHEMA = {
699+
sys_audit_log: SINGLE_TENANT.sys_audit_log,
700+
sys_activity: SINGLE_TENANT.sys_activity,
701+
sys_user_preference: ['id', 'user_id', 'key', 'value'],
702+
crm_lead: ['id', 'name'],
703+
};
704+
const OWNER = 'u_alice';
705+
const PREF = { id: 'pref-1', user_id: OWNER, key: 'ui.recent', value: [] as unknown[] };
706+
const rowsOn = (created: CapturedRow[], table: string, object: string) =>
707+
created.filter((c) => c.object === table && c.row.object_name === object);
708+
709+
const OWN_WRITES = [
710+
{ event: 'afterInsert', action: 'create', ctx: { input: { id: PREF.id }, result: PREF } },
711+
{
712+
event: 'afterUpdate',
713+
action: 'update',
714+
ctx: { input: { id: PREF.id }, previous: PREF, result: { ...PREF, value: [{ id: 'acc-1' }] } },
715+
},
716+
{ event: 'afterDelete', action: 'delete', ctx: { input: { id: PREF.id }, previous: PREF, result: { id: PREF.id } } },
717+
] as const;
718+
719+
for (const w of OWN_WRITES) {
720+
it(`the owner's own ${w.action} writes the sys_audit_log row and no sys_activity row`, async () => {
721+
const { engine, fire, created } = makeEngine(SCHEMA);
722+
installAuditWriters(engine as any, 'test.audit');
723+
724+
await fire(w.event, { object: 'sys_user_preference', ...w.ctx, session: { userId: OWNER } });
725+
726+
const ledger = rowsOn(created, 'sys_audit_log', 'sys_user_preference');
727+
expect(ledger).toHaveLength(1);
728+
expect(ledger[0].row.action).toBe(w.action);
729+
expect(ledger[0].row.user_id).toBe(OWNER);
730+
expect(ledger[0].row.record_id).toBe(PREF.id);
731+
expect(created.filter((c) => c.object === 'sys_activity')).toHaveLength(0);
732+
});
733+
}
734+
735+
it('CONTROL: the same user\'s write to an ordinary audited object keeps its activity row', async () => {
736+
const { engine, fire, created } = makeEngine(SCHEMA);
737+
installAuditWriters(engine as any, 'test.audit');
738+
739+
await fire('afterUpdate', {
740+
object: 'crm_lead',
741+
input: { id: 'lead-1' },
742+
previous: { id: 'lead-1', name: 'Acme' },
743+
result: { id: 'lead-1', name: 'Acme Corp' },
744+
session: { userId: OWNER },
745+
});
746+
747+
expect(rowsOn(created, 'sys_audit_log', 'crm_lead')).toHaveLength(1);
748+
const activity = rowsOn(created, 'sys_activity', 'crm_lead');
749+
expect(activity).toHaveLength(1);
750+
expect(activity[0].row.actor_id).toBe(OWNER);
751+
});
752+
753+
const OTHER_WRITERS = [
754+
// An organization admin editing the row from Setup's User Preferences grid.
755+
{ who: 'a write by another user (an admin)', session: { userId: 'u_admin' }, previous: PREF, result: PREF, actor: 'u_admin' },
756+
// A system or service write: no acting user at all.
757+
{ who: 'a system write with no user', session: { isSystem: true }, previous: PREF, result: PREF, actor: null },
758+
// The owner moving the row to someone else: not purely their own row.
759+
{
760+
who: 'the owner moving the row to another user',
761+
session: { userId: OWNER },
762+
previous: PREF,
763+
result: { ...PREF, user_id: 'u_bob' },
764+
actor: OWNER,
765+
},
766+
] as const;
767+
768+
for (const w of OTHER_WRITERS) {
769+
it(`CONTROL: on a user's preference row, ${w.who} keeps its activity row`, async () => {
770+
const { engine, fire, created } = makeEngine(SCHEMA);
771+
installAuditWriters(engine as any, 'test.audit');
772+
773+
await fire('afterUpdate', {
774+
object: 'sys_user_preference',
775+
input: { id: PREF.id },
776+
previous: w.previous,
777+
result: { ...w.result, value: [{ id: 'acc-2' }] },
778+
session: w.session,
779+
});
780+
781+
expect(rowsOn(created, 'sys_audit_log', 'sys_user_preference')).toHaveLength(1);
782+
const activity = rowsOn(created, 'sys_activity', 'sys_user_preference');
783+
expect(activity).toHaveLength(1);
784+
expect(activity[0].row.actor_id).toBe(w.actor);
785+
});
786+
}
787+
});
788+
689789
describe('audit writers — enable.feeds server-side enforcement (#2707)', () => {
690790
const SCHEMA = {
691791
sys_audit_log: SINGLE_TENANT.sys_audit_log,

‎packages/plugins/plugin-audit/src/audit-writers.ts‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ export interface AuditWriterOptions {
155155
* Skip rules avoid recursion and noise:
156156
* - Never audit the audit/activity tables themselves.
157157
* - Never audit session/presence/auth tables (high-frequency, low value).
158+
* - A user's own write to their own per-user UI-state row writes the ledger
159+
* row and no activity row (`PER_USER_UI_STATE_OWNER_FIELD`).
158160
* - Read-only operations (`afterFind`) are not audited BY THIS WRITER. Since
159161
* #8992 the `read` action has its own writer in `read-audit.ts`, installed
160162
* separately and only over the objects a deployment opts in: record-detail
@@ -260,6 +262,67 @@ const SKIP_OBJECTS = new Set<string>([
260262
*/
261263
export const AUDIT_EXCLUDED_OBJECTS: string[] = [...SKIP_OBJECTS];
262264

265+
/**
266+
* [#22961] Per-user UI-state objects: the ledger row stays, and the activity
267+
* mirror is skipped for the owning user's OWN write only.
268+
*
269+
* The rule: what reaches a user's bell is another actor's act or something the
270+
* user needs, never the user's own UI state. A `sys_activity` row reaches the
271+
* bell (its Activity tab lists the newest rows the caller may read). A per-user
272+
* UI-state row is re-saved by the client as a side effect of using the
273+
* interface, and RLS keeps it private to its user, so the one reader of its
274+
* activity row is the user who just caused it. Mirrored, every navigation came
275+
* back as `Updated User Preference "ui.recent"`, and a fresh user's bell held
276+
* nothing else.
277+
*
278+
* - CLOSED and DECLARED: an object is a member only by being listed here,
279+
* mapped to the field that names its owning user. ⛔ Never a name heuristic.
280+
* No spec key means "per-user UI state"; `enable.activities: false` mutes
281+
* every writer, which is wider than this rule.
282+
* - OWN WRITES ONLY: the user the activity row would name (`actor_id`) must be
283+
* the owner on every side of the row the write carries. Another actor's
284+
* write keeps its activity row: an organization admin editing the row from
285+
* Setup's User Preferences grid (`modifyAllRecords` passes the self-only
286+
* RLS), a system write with no user, a write that moves the row to another
287+
* owner.
288+
* - ACTIVITY ONLY: the `sys_audit_log` row is written exactly as before. That
289+
* is why this is not `SKIP_OBJECTS`, which drops the ledger row too and is
290+
* the `AUDIT_EXCLUDED_OBJECTS` list the hook registration and `read-audit.ts`
291+
* share.
292+
*
293+
* Membership as measured: the console keeps all of its per-user UI state
294+
* (`ui.recent`, `ui.favorites`, `ui.flow.palette.recents`) on
295+
* `sys_user_preference`, and no other per-user UI-state object exists. A new
296+
* one joins by a line here.
297+
*/
298+
const PER_USER_UI_STATE_OWNER_FIELD: ReadonlyMap<string, string> = new Map([
299+
['sys_user_preference', 'user_id'],
300+
]);
301+
302+
/**
303+
* Whether this write is a user's own write to their own per-user UI-state row
304+
* ({@link PER_USER_UI_STATE_OWNER_FIELD}): the object is listed, an acting user
305+
* is known, and every side of the row that names an owner names that user.
306+
* A side without the owner field (a delete's result is often just `{ id }`) is
307+
* not evidence either way; no side naming an owner at all answers `false`.
308+
*/
309+
function isOwnUiStateWrite(
310+
object: string,
311+
actorUserId: string | undefined,
312+
before: unknown,
313+
after: unknown,
314+
): boolean {
315+
const ownerField = PER_USER_UI_STATE_OWNER_FIELD.get(object);
316+
if (!ownerField || !actorUserId) return false;
317+
const owners: string[] = [];
318+
for (const side of [before, after]) {
319+
if (!side || typeof side !== 'object') continue;
320+
const owner = (side as Record<string, unknown>)[ownerField];
321+
if (owner !== undefined && owner !== null) owners.push(String(owner));
322+
}
323+
return owners.length > 0 && owners.every((owner) => owner === actorUserId);
324+
}
325+
263326
/** Fields that are noise in diffs (always change, never user-meaningful). */
264327
const NOISE_FIELDS = new Set<string>([
265328
'updated_at',
@@ -2012,14 +2075,19 @@ export function installAuditWriters(
20122075
// lever for activity-row growth (ADR-0057). The compliance audit row is
20132076
// NOT gated — sys_audit_log capture stays unconditional.
20142077
const activitiesEnabled = getObjectDef(ctx.object)?.enable?.activities !== false;
2078+
// [#22961] A user's own write to their own per-user UI-state row is not
2079+
// mirrored either (PER_USER_UI_STATE_OWNER_FIELD states the rule). Judged
2080+
// on `userId`, the same value `actor_id` would carry. The ledger row is
2081+
// untouched.
2082+
const mirrorsActivity = activitiesEnabled && !isOwnUiStateWrite(ctx.object, userId, before, after);
20152083
// [#22510, #22527] The name and avatar of the user `actor_id` names — the
20162084
// SAME `userId`, so the columns cannot disagree about who acted — from one
20172085
// memoized `sys_user` read. Read only when an activity row will be
20182086
// written. No user ⇒ no name and no avatar: ADR-0118 D1 keeps the system
20192087
// actor `null`, and rendering it as "System" (with its icon) is the UI's
20202088
// rule, not data this writer persists.
20212089
const actor: ActorSnapshot =
2022-
activitiesEnabled && userId ? await resolveActorSnapshot(api, userId) : NO_ACTOR_SNAPSHOT;
2090+
mirrorsActivity && userId ? await resolveActorSnapshot(api, userId) : NO_ACTOR_SNAPSHOT;
20232091

20242092
const activityRow: Record<string, any> = {
20252093
type: activityType,
@@ -2055,7 +2123,7 @@ export function installAuditWriters(
20552123
activityRow.organization_id = tenantId ?? null;
20562124
}
20572125

2058-
const activityRowToWrite = activitiesEnabled ? activityRow : undefined;
2126+
const activityRowToWrite = mirrorsActivity ? activityRow : undefined;
20592127
// [#21262] The ledger row is written first; the writer advances this
20602128
// before the activity row, so after a throw it names the refused table.
20612129
const progress: AuditTrailWriteProgress = { writing: 'sys_audit_log' };

0 commit comments

Comments
 (0)