Skip to content

Commit 6d1c659

Browse files
fix(plugin-audit)!: a comment's display name is stamped from the caller's session, like its author id (#29737)
Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: none — no skill or doc teaches a client to write a comment's display fields; the one docs example (`content/docs/frontend/plugins/plugin-detail.mdx`) reads `author_name`, which is unchanged. ## 维护者速读 - **改了什么**:评论的显示名与头像(`author_name` / `author_avatar_url`)改由服务端在创建时按当前会话用户自己的资料(`sys_user` 的名称与 `image`)写入,与 `author_id` 同源;编辑时这两个字段只读,保持创建时的值。系统上下文写入(种子、导入、迁移)不变。 - **为什么改**:评论的显示身份应与作者 id 一样是服务端事实,读者看到的名字只能是作者本人的。 - **风险与回滚**:收窄(请求里带的这两个字段不再落库);控制台无需改动。直接调用 `installCommentAccessHooks` 而不传解析器的宿主,新评论不带显示名(安装时 warn 一次)。回滚即 revert 本 PR。 - **先看**:`packages/plugins/plugin-audit/src/comment-access-hooks.ts`(创建钩子末尾的盖章段与 `authorIdentityOf`)、`packages/plugins/plugin-audit/src/objects/sys-comment.object.ts`(两字段 `readonly`)、`packages/plugins/plugin-audit/src/audit-plugin.ts`(解析器接线)。 - **两分钟验证**:`pnpm --filter @objectstack/dogfood exec vitest run test/comment-author-display-identity.dogfood.test.ts`,预期 `Tests 3 passed (3)`。 - **需要你做的**:无,席位落地。 ## What changes A comment's display name is stamped from the caller's session, like its author id. - **Create.** The `sys_comment` create hook (`installCommentAccessHooks`) assigns `author_name` and `author_avatar_url` from the session user's own `sys_user` profile, the title column and `image`, after the comment is admitted, whether or not the request carries them. A user whose profile cannot be read is stored with neither. The resolution is the audit writers' actor snapshot, the one the record history's actor columns are written with: `installAuditWriters` now returns a handle with `resolveActorSnapshot(api, userId)`, and `AuditPlugin` passes it to `installCommentAccessHooks` as a new optional sixth argument, `resolveAuthorIdentity`. - **Update.** Both fields declare `readonly: true` on `sys_comment`, so the engine's update-side strip drops a non-system caller's value and reports it in `droppedFields` and `X-ObjectStack-Dropped-Fields`. The write is never refused, and an edit, the author's or a parent editor's, never changes whose name a comment shows. - **Unchanged.** `author_id` keeps its rules. System-context writes keep the values they supply. Stored comments keep what they were written with. The console's create payload is accepted as before, and its display fields are assigned over. - **Also.** A comment posted with no display name (REST, MCP, agent) now shows its author's name instead of none, and the @mention notification title names that author. ## Tests All readings below are at `dde4c9f4b` unless a line says otherwise. - New `packages/plugins/plugin-audit/src/comment-author-display-identity.integration.test.ts`, real kernel, ObjectQL, AuditPlugin and SQLite, with the platform's `sys_user`: the caller's own name and avatar are stored whatever the request carries; a comment with none sent is stamped (the control); an unreadable profile stores neither; an update strips both and reports them; a parent editor's edit keeps the author's; a system write keeps its own. - New `packages/qa/dogfood/test/comment-author-display-identity.dogfood.test.ts`, at the REST data door of a booted stack: the served comment shows the caller's own name and avatar whatever the request carries; control: an ordinary comment shows its caller's own name, for each of two callers; a PATCH naming both fields answers 200, `droppedFields` names them with reason `readonly`, the body edit lands, and the served identity is unchanged. - Repaired `packages/plugins/plugin-audit/src/audit-write-locale-settings-seam.integration.test.ts`: it named its mention actor through a display name in the create request, which this change no longer stores; it now seeds the author's `sys_user` profile. Its four assertions are unchanged. <details><summary>plugin-audit: full suite and typecheck</summary> ``` pnpm --filter @objectstack/plugin-audit test Test Files 69 passed (69) Tests 1115 passed (1115) pnpm --filter @objectstack/plugin-audit typecheck check:test-typecheck: OK — 0 file(s) / 0 error(s) / 0 pinned signature(s) os-verify-lock: VERDICT command-exit 0 ``` Before the fixture repair the same suite read `1 failed | 1114 passed`: the locale seam pin expected `Ada mentioned you` and received `You were mentioned`. </details> <details><summary>dogfood: full battery and typecheck</summary> ``` pnpm --filter @objectstack/dogfood test Test Files 400 passed | 3 skipped (403) Tests 3300 passed | 28 skipped (3328) os-verify-lock: VERDICT command-exit 0 · held the lock 1722s pnpm --filter @objectstack/dogfood typecheck os-verify-lock: VERDICT command-exit 0 ``` </details> ## Acceptance notes - The display identity is a snapshot taken at creation, read through the actor snapshot (at most one `sys_user` read per user per 30 s window). A rename or a new avatar shows on comments created after the window; existing comments keep what they were created with, as record history entries do. - A host that installs the comment hooks itself without `resolveAuthorIdentity` stores no display identity on new comments, never the request's, and is told once at install, at `warn`. ## Verification <details><summary>Ablation of the create stamp (one shot, both pins)</summary> The two assignment lines of the stamp were replaced by a no-op carrying a marker, through `scripts/ablation-replace.mjs` (anchor 1 to 0, blob `633abc66558f` to `1b752dcfe3fb`), `@objectstack/plugin-audit` rebuilt, and `scripts/ablation-dist-preflight.mjs` read the marker in 2 built files before either suite ran (the dogfood suite resolves plugin-audit through `dist/`). ``` mutated: integration Tests 5 failed | 1 passed (6) (the system-write pin stays green: that path is untouched) dogfood Tests 3 failed (3) restored: blob == HEAD (633abc6), git diff HEAD empty; rebuilt; preflight --absent: marker absent from all 6 built files, tree clean integration Tests 6 passed (6) dogfood Tests 3 passed (3) ``` </details> <details><summary>Derived gates (dispatch-gates --commands, 67 families) at dde4c9f</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives 67 commands for this diff. All 67 were run at `dde4c9f4b`, each exit code captured before any pipe; 66 exited 0. `pnpm check:type-check-debt` exited 3 there (its closure build did not get a turn on the shared verify lock) and was re-run under the lock; its reading is in the line below. `--ran` reconciliation: 67 derived, 67 accounted for. ``` bash scripts/pm/os-verify-lock.sh -c 'pnpm check:type-check-debt' check-type-check-coverage: OK — 126/129 workspace packages type-checked (plus the root), 0 in the DEBT ledger check-type-check-coverage --re-measure: OK — 0 raw tsc error(s) total, none above its recorded number. os-verify-lock: VERDICT command-exit 0 ``` `node scripts/check-plugin-teardown-shape.mjs --self-test` first refused on this shallow clone (its pinned control commit was absent); after fetching that one commit it exits 0, 48 cases. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 63f962f commit 6d1c659

8 files changed

Lines changed: 418 additions & 5 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
---
4+
5+
fix(plugin-audit)!: a comment's display name is stamped from the caller's session, like its author id
6+
7+
Clause-②: no (narrowing) — on a `sys_comment` write by a non-system caller, an `author_name` / `author_avatar_url` the request carries is not stored: on create both are the session user's own, and on update both are read-only and dropped (reported in `droppedFields`). Nothing is widened.
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, and no stored row is read, rewritten, converted or dropped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is a runtime accept set on two platform-written columns of one platform object, `sys_comment.author_name` and `author_avatar_url`, whose values now come from the user's own profile; the remedy for a user who wants another name or picture shown is editing that profile, which no ledger entry can derive. The other categories are closed on facts: the bumped package publishes (not unpublished); no ADR-0087 id covers these columns and this diff adds none (not registered, not already-registered); `installAuditWriters` gains a return value and `installCommentAccessHooks` an optional parameter, so no export is retired and no interface or type narrows (not runtime-export-retired, runtime-interface-only or type-surface-only). -->
11+
12+
**BREAKING** accept-set narrowing on `sys_comment` writes (`@objectstack/plugin-audit`), served by every data door (REST, MCP, the console).
13+
14+
- **Now.** A comment's display identity is the server's, the same as its `author_id`. On create, `author_name` and `author_avatar_url` are the session user's own display name and avatar, read from their `sys_user` profile (the title column and `image`) through the same resolution the record history's actor columns are written with, and assigned whether or not the request carries them. A user whose profile cannot be read is stored with neither. On update both fields are `readonly`: a non-system caller's value is dropped and reported in `droppedFields` and the `X-ObjectStack-Dropped-Fields` header, the rest of the write lands, and the write is never refused, so an edit, the author's or a parent editor's, never changes whose name a comment shows.
15+
- **Unchanged.** `author_id` keeps its rules. System-context writes (seeds, imports, migrations) keep the values they supply, on create and on update. Comments already stored keep what they were written with. The console needs no change: the create payload it sends is accepted and its display fields are assigned over.
16+
- **Also.** A comment posted without a display name (a REST, MCP or agent client that sends none) now shows its author's name, where it used to show none.
17+
- **Hosts that compose the hooks themselves.** `installAuditWriters` now returns a handle whose `resolveActorSnapshot(api, userId)` is that resolution, and `installCommentAccessHooks` takes it as an optional sixth argument, `resolveAuthorIdentity(ctx, userId)`. `AuditPlugin` wires the two together. A host that installs the comment hooks without it stores no display identity on new comments (readers show them as unknown) and is told so once, at install, at `warn`.
18+
19+
**The remedy.** None for the console or for a client that sends no display identity. A user changes the name and picture their comments show by editing their own profile (`sys_user.name` / `image`); a comment keeps the values it was created with.

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ export class AuditPlugin implements Plugin {
437437
return undefined;
438438
}
439439
};
440-
installAuditWriters(engine as any, this.name, {
440+
const auditWriters = installAuditWriters(engine as any, this.name, {
441441
getMessaging,
442442
getI18n,
443443
getLocale,
@@ -515,6 +515,12 @@ export class AuditPlugin implements Plugin {
515515
return null;
516516
}
517517
},
518+
// A comment's display identity (author name and avatar) is the
519+
// session user's own, read by the same resolution the record
520+
// history's actor columns are written with.
521+
auditWriters
522+
? (hookCtx, userId) => auditWriters.resolveActorSnapshot(hookCtx?.api, userId)
523+
: undefined,
518524
);
519525
if (typeof (engine as any).registerMiddleware === 'function') {
520526
// [ADR-0139 E7] The thread gate writes one ReadableIds leaf per parent

‎packages/plugins/plugin-audit/src/audit-write-locale-settings-seam.integration.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
88
import { ObjectKernel } from '@objectstack/core';
99
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
1010
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
11+
import { SysUser } from '@objectstack/platform-objects/identity';
1112
import { AuditPlugin } from './audit-plugin.js';
1213
import { singlePostureTenancy } from './__tests__/single-posture-tenancy.js';
1314
import { installDefaultOrganization } from './default-organization.testkit.js';
@@ -106,7 +107,6 @@ describe('audit writers — a localization change reaches the very next @mention
106107
{
107108
thread_id: `biz_task:${taskId}`,
108109
author_id: 'u_admin',
109-
author_name: 'Ada',
110110
body: `note ${comments}`,
111111
mentions: '["u_bob"]',
112112
},
@@ -129,10 +129,14 @@ describe('audit writers — a localization change reaches the very next @mention
129129
engine.registerDriver(driver, true);
130130
engine.registry.registerObject(sysSetting as any, HARNESS_PACKAGE);
131131
engine.registry.registerObject(task as any, HARNESS_PACKAGE);
132+
// The mention's actor is named from the author's own profile: a comment's
133+
// display name is stamped from the session, like its author id.
134+
engine.registry.registerObject(SysUser as any, HARNESS_PACKAGE);
132135
await engine.syncSchemas();
133136
// A real `single` install's Default Organization: the system writes below
134137
// derive it (ADR-0131 D9), as a fresh NOT NULL tenant column requires (D1).
135138
await installDefaultOrganization(engine);
139+
await engine.insert('sys_user', { id: 'u_admin', name: 'Ada', email: 'ada@seam.test' }, SYSTEM as any);
136140

137141
sql = async (statement, bindings) => {
138142
const result = await (driver as unknown as { execute(s: string, b: unknown[]): Promise<unknown> }).execute(

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

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,29 @@ export interface AuditI18nSurface {
140140
t(key: string, locale: string, params?: Record<string, unknown>): string;
141141
}
142142

143+
/**
144+
* A user's display identity as the platform holds it: the `sys_user` title
145+
* column (the display name) and `sys_user.image` (the avatar URL), each `null`
146+
* where it cannot be read. The rule is `resolveActorSnapshot`'s, inside
147+
* {@link installAuditWriters}.
148+
*/
149+
export interface AuditActorSnapshot {
150+
name: string | null;
151+
avatarUrl: string | null;
152+
}
153+
154+
/** What {@link installAuditWriters} hands back to the plugin that composes it. */
155+
export interface AuditWritersHandle {
156+
/**
157+
* The display identity of `userId`, read from `sys_user` with the system's
158+
* own read through `api` (a hook context's `api`) — the one resolution the
159+
* `sys_activity` actor columns are written with, so every other platform
160+
* writer of a user's display identity asks it rather than keeping a second
161+
* one. It never rejects: an unreadable identity answers `null` for both.
162+
*/
163+
resolveActorSnapshot(api: unknown, userId: string): Promise<AuditActorSnapshot>;
164+
}
165+
143166
/** Options for {@link installAuditWriters}. */
144167
export interface AuditWriterOptions {
145168
/**
@@ -1239,8 +1262,8 @@ export function installAuditWriters(
12391262
engine: any,
12401263
packageId = 'com.objectstack.audit',
12411264
opts: AuditWriterOptions = {},
1242-
): void {
1243-
if (!engine || typeof engine.registerHook !== 'function') return;
1265+
): AuditWritersHandle | undefined {
1266+
if (!engine || typeof engine.registerHook !== 'function') return undefined;
12441267

12451268
const getMessaging = opts.getMessaging ?? (() => undefined);
12461269
const getI18n = opts.getI18n ?? (() => undefined);
@@ -1733,7 +1756,7 @@ export function installAuditWriters(
17331756
* rows written after the window; rows already written keep what they were
17341757
* written with, which is what a snapshot is.
17351758
*/
1736-
type ActorSnapshot = { name: string | null; avatarUrl: string | null };
1759+
type ActorSnapshot = AuditActorSnapshot;
17371760
const NO_ACTOR_SNAPSHOT: ActorSnapshot = { name: null, avatarUrl: null };
17381761
/** `sys_user`'s profile-image column (ADR-0092 Tier 1: "`image` — avatar URL"). */
17391762
const USER_AVATAR_FIELD = 'image';
@@ -2668,6 +2691,10 @@ export function installAuditWriters(
26682691
// judged and notified like one written with it. The registration is what
26692692
// has the engine bind `previous` on this object's update.
26702693
engine.registerHook('afterUpdate', writeCommentMentions, { object: 'sys_comment', packageId });
2694+
2695+
// The comment author's display identity is stamped from this same
2696+
// resolution (`installCommentAccessHooks`, wired by `AuditPlugin`).
2697+
return { resolveActorSnapshot };
26712698
}
26722699

26732700
/**

‎packages/plugins/plugin-audit/src/comment-access-hooks.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@
2323
* verified with a caller-scoped `findOne`, so the parent's own
2424
* OWD/sharing, RLS and object-level CRUD all apply. `author_id` is
2525
* server-stamped from the session; a client-supplied value never wins.
26+
* So is the display identity beside it: `author_name` and
27+
* `author_avatar_url` are the session user's own name and avatar
28+
* ({@link CommentAuthorIdentityResolver}), never the client's, and both
29+
* are read-only on update (`objects/sys-comment.object.ts`).
2630
* * `beforeUpdate` / `beforeDelete`: the caller must be the comment's
2731
* AUTHOR, or hold EDIT on the parent record (see "Parent EDIT" below) —
2832
* the attachment kit's uploader-or-parent-editor rule. A multi-row write
@@ -221,6 +225,24 @@ export interface CommentAccessLogger {
221225
debug?(msg: string, meta?: unknown): void;
222226
}
223227

228+
/** A comment author's display identity: the name and avatar URL the platform
229+
* holds for that user, each `null` where it cannot be read. */
230+
export interface CommentAuthorIdentity {
231+
name: string | null;
232+
avatarUrl: string | null;
233+
}
234+
235+
/**
236+
* Resolves the display identity of the user a comment is stamped with, given
237+
* the write's hook context. `AuditPlugin` wires the audit writers' actor
238+
* snapshot (`AuditWritersHandle.resolveActorSnapshot`), the resolution the
239+
* `sys_activity` actor columns are written with, so a user reads as the same
240+
* name in a comment and in the record's history.
241+
*/
242+
export type CommentAuthorIdentityResolver = (ctx: any, userId: string) => Promise<CommentAuthorIdentity>;
243+
244+
const NO_AUTHOR_IDENTITY: CommentAuthorIdentity = { name: null, avatarUrl: null };
245+
224246
/** Must match `AuditPlugin.name` — hooks are removable by owning package. */
225247
const PACKAGE_ID = 'com.objectstack.audit';
226248
const SYSTEM_CTX = { isSystem: true } as const;
@@ -709,14 +731,45 @@ async function callerCanRead(ctx: any, target: CommentThreadTarget): Promise<boo
709731
* asked on a record-sharing abstention instead; absent too, a
710732
* `controlled_by_parent` parent meets no master check here, exactly as its own
711733
* update meets none.
734+
*
735+
* `resolveAuthorIdentity` answers the display identity (name, avatar) a new
736+
* comment is stamped with, for the session's user. Absent, a comment is stored
737+
* with neither, never with the client's: the display identity is the server's
738+
* or none. That composition is said once, here, at install.
712739
*/
713740
export function installCommentAccessHooks(
714741
engine: CommentAccessEngine,
715742
getSharing: () => CommentSharingLike | null | undefined,
716743
logger: CommentAccessLogger,
717744
messageTranslator?: () => ValidationMessageTranslator | undefined,
718745
getSecurity?: () => CommentSecurityLike | null | undefined,
746+
resolveAuthorIdentity?: CommentAuthorIdentityResolver,
719747
): void {
748+
if (!resolveAuthorIdentity) {
749+
logger.warn(
750+
'[audit] comment access: no author identity resolver is composed — new comments store no author_name / ' +
751+
'author_avatar_url (readers show them as unknown). AuditPlugin wires one; a host calling ' +
752+
'installCommentAccessHooks directly passes `resolveAuthorIdentity`.',
753+
);
754+
}
755+
756+
/** The display identity of the session's user, the server's own answer: a
757+
* session with no user, a resolver that is absent or fails, and a value that
758+
* is not a non-blank string all answer `null`, never the client's value. */
759+
const authorIdentityOf = async (ctx: any): Promise<CommentAuthorIdentity> => {
760+
const userId: unknown = ctx.session.userId;
761+
if ((typeof userId !== 'string' && typeof userId !== 'number') || String(userId) === '') return NO_AUTHOR_IDENTITY;
762+
if (!resolveAuthorIdentity) return NO_AUTHOR_IDENTITY;
763+
let identity: CommentAuthorIdentity | null | undefined;
764+
try {
765+
identity = await resolveAuthorIdentity(ctx, String(userId));
766+
} catch {
767+
return NO_AUTHOR_IDENTITY;
768+
}
769+
const text = (value: unknown): string | null => (typeof value === 'string' && value.trim() ? value : null);
770+
return { name: text(identity?.name), avatarUrl: text(identity?.avatarUrl) };
771+
};
772+
720773
/** May the caller EDIT the parent record behind `target`? The composition
721774
* "Parent EDIT" in the module header sets out. [#24646] Where the security
722775
* service serves `explain`, the parent's own by-id update judgement decides
@@ -891,6 +944,16 @@ export function installCommentAccessHooks(
891944
target.object,
892945
);
893946
}
947+
948+
// The display identity is the server's too, exactly as `author_id`
949+
// above: the name and avatar a reader is shown are the session user's
950+
// own, assigned over whatever the client sent, and assigned when it sent
951+
// nothing. Asked only once the comment is admitted, so a refused write
952+
// reads no identity. On update both are read-only
953+
// (`objects/sys-comment.object.ts`), so they stay the creator's.
954+
const identity = await authorIdentityOf(ctx);
955+
data.author_name = identity.name;
956+
data.author_avatar_url = identity.avatarUrl;
894957
};
895958
engine.registerHook('beforeInsert', onInsert, { object: COMMENT_OBJECT, packageId: PACKAGE_ID });
896959
engine.registerHook('beforeInsert', onInsert, { object: COMMENT_REACTION_OBJECT, packageId: PACKAGE_ID });

0 commit comments

Comments
 (0)