feat(plugin-audit): the platform stamps sys_comment is_edited / edited_at when a comment's body is edited

.changeset/23124-comment-edit-stamp.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/plugin-audit': minor
+---
+
+`sys_comment` edit marks are platform-written: an update that changes a comment's `body` stamps `is_edited: true` and `edited_at` from the server clock, and a client can no longer set or clear either field.
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: `sys_comment` keeps both fields with their names and types, no spec key, authorable spelling, export or stored shape is added, removed, renamed or re-shaped, and a row stored before this change keeps its bytes, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is one runtime write door: a client's value for two platform-written fields no longer lands, and a predicate body update whose matched rows disagree is refused; the remedy is in the request a caller sends to that door, never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no exported declaration is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
+
+**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
+
+- **Stamped by the platform.** The comment's `beforeUpdate` sets both fields when the update's `body` differs from the stored body, for every caller, the way the engine stamps `updated_at`. An update that does not name `body`, or sends the stored text again beside new `mentions`, is not an edit and stamps nothing. A create is not an edit either.
+- **Read-only at the data door.** `is_edited` and `edited_at` now declare `readonly: true`. On an update that edits the body, the stamp replaces whatever the client sent; on any other update a non-system caller's value is dropped and reported in the response's `droppedFields`; on a create the platform sets both to "not edited" over a client value. Nothing is refused for carrying the fields — a client that still sends them on every edit, as the console released before this change does, keeps working and its values are ignored. Read both fields back from the row instead of writing them.
+- **Predicate updates.** A `multi: true` update is stamped per matched row. One whose matched rows disagree — some bodies change, some already hold the new text — is now refused whole with `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` (400) and writes nothing, because one SET clause cannot mark some rows and not others. Narrow the predicate to the rows whose text changes, or update by id.
+- A `preserveAudit` (historical import) update keeps a mark it supplies, as it keeps `updated_at`.

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

@@ -71,8 +71,11 @@ function install(opts: {
   const parentReads: Array<{ object: string; options: any }> = [];
   const parentFind = readableParentFind(opts.readable ?? [], parentReads);
   const engine: CommentAccessEngine = {
+    // The FIRST handler per event is the authorization gate these tests drive:
+    // `beforeUpdate` also carries the edit stamp (#23124), registered after it
+    // and pinned on the real engine in `comment-edit-stamp.integration.test.ts`.
     registerHook: (event, handler) => {
-      hooks.set(event, handler as any);
+      if (!hooks.has(event)) hooks.set(event, handler as any);
     },
     find: async (object, options: any) => {
       if (object !== 'sys_comment') return parentFind(object, options);
@@ -644,6 +647,10 @@ const COMMENT_FIELDS = {
   thread_id: { name: 'thread_id', label: 'Thread', type: 'text' as const },
   author_id: { name: 'author_id', label: 'Author', type: 'text' as const },
   body: { name: 'body', label: 'Body', type: 'text' as const },
+  // The edit stamp `installCommentAccessHooks` also registers (#23124) writes
+  // these on a body change; declared as `sys_comment` declares them.
+  is_edited: { name: 'is_edited', label: 'Edited', type: 'boolean' as const, readonly: true },
+  edited_at: { name: 'edited_at', label: 'Edited At', type: 'datetime' as const, readonly: true },
 };
 const sysCommentObject = { name: 'sys_comment', label: 'Comment', fields: COMMENT_FIELDS };
 const crmOpportunityObject = {

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

@@ -39,6 +39,10 @@
  *      platform's not-visible refusal instead of the named one (#21755); see
  *      {@link refuseNotVisible}. The named refusal is what a caller who can
  *      read the parent, but may not edit it, receives.
+ *    * The edit stamp (#23124, ruling A): a SECOND `beforeUpdate` handler, not
+ *      part of the authorization above, sets `is_edited` / `edited_at` when the
+ *      body changes — see {@link stampCommentBodyEdit}. The create hook sets
+ *      both to "not edited" over a client value.
  *  - {@link installCommentReadVisibility} — the read side: a
  *    `find`/`findOne`/`count`/`aggregate` middleware that intersects the query
  *    with the threads whose parent record the caller can actually read.
@@ -319,6 +323,59 @@ function refuseNotVisible(
   throw err;
 }
 
+/**
+ * [#23124, maintainer ruling A] The platform marks a comment edited: when an
+ * update's `body` differs from the stored body, set `is_edited: true` and
+ * `edited_at` to the server clock. Both fields are `readonly` on
+ * `sys_comment`, so on an update by a non-system caller this hook is their one
+ * writer, as the create hook is on a create (`objects/sys-comment.object.ts`,
+ * "Platform-written").
+ *
+ * What counts as an edit is the BODY and nothing else: an update that does not
+ * name `body`, or names the stored text again (the console rewrites `mentions`
+ * beside it), stamps nothing. A row with no readable pre-image is stamped —
+ * the engine binds `previous` on both update paths, and a mark that cannot be
+ * proven unneeded is the safe side of a reader's trust signal.
+ *
+ * Every caller is stamped, system included, the way the engine's `updated_at`
+ * stamp is: the mark records that the text changed after it was posted, not
+ * who was allowed to change it (the authorization handler is separate and
+ * exempts system callers; this one does not). The one exception is the
+ * engine's own: a `preserveAudit` write (a historical import) keeps a value it
+ * supplies, as `updated_at` does, and is stamped only where it supplies none.
+ *
+ * Why the stamp is an assignment and the client's value needs no handling here:
+ * the engine hides a non-system caller's `readonly` values from the hooks and
+ * strips them after, and keeps a value a hook ASSIGNED (`hookWrittenKeys`). So
+ * on an edit the stamp lands whatever the client sent, and without one the
+ * client's value is dropped and reported as `droppedFields`.
+ *
+ * A predicate (`multi: true`) update dispatches this handler once per matched
+ * row, with that row as `previous`, over ONE shared payload. If every row's
+ * body changes, every row is stamped; if none does, none is. A batch whose rows
+ * disagree is refused whole by the engine (`MULTI_UPDATE_HOOK_KEY_DIVERGENCE`,
+ * 400, nothing written) — one SET clause cannot mark some rows and not others,
+ * and stamping all of them would mark text that never changed.
+ */
+async function stampCommentBodyEdit(ctx: any): Promise<void> {
+  const data: unknown = ctx?.input?.data;
+  if (!data || typeof data !== 'object' || Array.isArray(data)) return;
+  const payload = data as Record<string, unknown>;
+  if (!Object.prototype.hasOwnProperty.call(payload, 'body')) return;
+  const previous: unknown = ctx?.previous;
+  if (previous && typeof previous === 'object' && Object.is((previous as Record<string, unknown>).body, payload.body)) {
+    return;
+  }
+  const now = new Date().toISOString();
+  if (ctx?.session?.preserveAudit === true) {
+    payload.is_edited = payload.is_edited ?? true;
+    payload.edited_at = payload.edited_at ?? now;
+    return;
+  }
+  payload.is_edited = true;
+  payload.edited_at = now;
+}
+
 function asIdList(id: unknown): Array<string | number> | null {
   if (typeof id === 'string' || typeof id === 'number') return [id];
   if (id && typeof id === 'object' && Array.isArray((id as any).$in)) {
@@ -539,6 +596,14 @@ export function installCommentAccessHooks(
     // spoofable — a caller could post as anyone, then "author-delete" it.
     if (ctx.session.userId) data.author_id = ctx.session.userId;
 
+    // [#23124] A new comment is not an edited one, whatever the client sent.
+    // The engine's create-side `readonly` strip leaves `sys_` objects to their
+    // own guards, so this is the guard: the platform's value, assigned over a
+    // client's, exactly as `author_id` above. Only a key the client named is
+    // touched; an absent one takes the field's default.
+    if (Object.prototype.hasOwnProperty.call(data, 'is_edited')) data.is_edited = false;
+    if (Object.prototype.hasOwnProperty.call(data, 'edited_at')) data.edited_at = null;
+
     const target = parseCommentThreadId(data.thread_id);
     if (!target) {
       forbid(
@@ -729,6 +794,13 @@ export function installCommentAccessHooks(
     { object: 'sys_comment', packageId: PACKAGE_ID, dispatchUnscopedMultiWrite: true },
   );
 
+  // ── Update: the platform stamps an edit of the body (#23124) ─────────
+  // Its own registration, deliberately: the authorization handler above
+  // returns early for system and context-less callers and declares the
+  // whole-operation dispatch, and the stamp wants neither — see
+  // {@link stampCommentBodyEdit}.
+  engine.registerHook('beforeUpdate', stampCommentBodyEdit, { object: COMMENT_OBJECT, packageId: PACKAGE_ID });
+
   // ── Delete: author or parent editor ─────────────────────────────────
   engine.registerHook(
     'beforeDelete',