Repository navigation
Commit 29a163f
feat(plugin-audit): sys_comment_reaction, the reactor's own record of an emoji reaction (#22603)
Fixes #22566
Clause-②: yes
The additive half of ruling A, amended, on #22505 (director record
`6091550406`, delegated judgment `6091621568`), scoped by triage
`6092508551` and claim `6092717444`. A new object,
`sys_comment_reaction`, holds one emoji reaction per row, and the row
belongs to the user who reacted. The retirement of
`sys_comment.reactions` is #22573 and is not touched here: the field,
its description and its bundles are byte-identical. #22500 remains open;
it waits on the full ruling, not on this PR.
**Why the first line is `Part of`.** One piece of the card's scope
cannot be built as the card words it, so it is back with the seat as a
decision (see "Who grants the bits" below). Everything else the card
scopes is in this diff. If the decision lands on option B, nothing here
changes and the first line can be rewritten to close the card.
## What this adds
- **The object**
(`plugin-audit/src/objects/sys-comment-reaction.object.ts`, registered
beside `sys_comment` in `AuditPlugin.init` and provisioned with it).
Fields: `comment_id` (text, at most 255 characters), `emoji` (text, at
most 64), `user_id` (lookup to `sys_user`), `created_at`. `apiMethods:
['get', 'list', 'create', 'delete']`; `userActions: { edit: false,
import: false }`; `feeds: false`, `activities: false`. No
`sharingModel`, and the audit columns are injected, so `created_by`
exists and is stamped.
- **Read rule.** A reaction is readable exactly when its comment is.
Details below.
- **Create rule.** A caller reacts only to a comment it can read; a
comment it cannot read, or one that does not exist, is refused with `403
RECORD_NOT_ACCESSIBLE` (the comment gate's own code, no new code).
`user_id` is stamped from the session, and a value the client sends is
replaced. A session with no user is refused.
- **Delete rule.** No new code. The row is the reactor's own, so the
platform's own-record floor, `owner_only_deletes` (`created_by ==
current_user.id`, positions `org_member`), is the whole rule: a member
deletes their own reaction and not another member's. No moderation-style
alternate match is contributed for reactions.
- **Update.** Not offered (`apiMethods`). A reaction is deleted and made
again.
- The tenancy census and the plugin-audit translation bundles are
regenerated by their generators (`platform-object-tenancy-census.mjs
--write`, `check-i18n-bundles.mjs --write --filter=audit`). The zh-CN,
ja-JP and es-ES leaves are the generator's source fills, with provenance
records.
## The read rule, and why this one
Measured options:
1. **`controlled_by_parent` on a `master_detail` to `sys_comment`.**
Rejected. ADR-0055 §3 makes every write to a detail require EDIT on its
master, and a member cannot edit another member's comment (the
author-only floor), so it would refuse exactly the reaction the ruling
admits. Its read half is also the wrong question: the master set comes
from the master's RLS and sharing filters, which never run the comment's
thread gate, because that gate is an engine middleware, not a row
policy.
2. **A copy of the thread rule on reactions.** Ruled out by the card.
3. **Chosen: ask the comment.** The comment gate's two installers in
`comment-access-hooks.ts` now answer for `sys_comment_reaction` too,
with the same handler registered on both objects. The reaction read gate
pre-scans the candidate `comment_id`s under system context, then hands
them to `resolveReadableParentIds` as one caller-scoped read of
`sys_comment`. That read runs everything a direct read of those comments
runs: the caller's grant on `sys_comment`, its row scope, and the thread
gate, which in turn asks the record each thread names. The result is
ANDed into the reaction query as `comment_id` `$in`. It covers `find`,
`findOne`, `count` and `aggregate`. Analytics routes the object through
the engine, because it carries an object middleware. The create gate
asks the same evaluator, so the create door never admits a reaction the
read door would hide.
Why one handler per seam and not one per object: the system and
context-less exemption each installer opens with is read once for both
objects. The `isSystem` census (`check:system-context-census`) stays at
123 read sites, with no new anchor needed.
## Uniqueness
`(comment_id, emoji, user_id)` is one row, held by a declared unique
index with its scope spelled out (`unique: 'organization'`, ADR-0120
D1). That is the platform's existing answer. A second identical reaction
is refused by the store: the engine envelope is `DUPLICATE_RECORD` 409,
and the data door answers it as `409 UNIQUE_VIOLATION`, pinned through
`mapDataError`. Two members reacting at once keep two rows. No new error
code.
## Why `comment_id` is an id column and not a lookup
Referential behaviour on delete runs as the caller
(`cascadeDeleteRelations` recurses with the caller's context; the
`cascade` arm is excluded from the referential-clear exemption by
ruling). A `cascade` lookup would make an author's delete of their own
comment delete other members' reactions under the author's authority,
and the own-record floor refuses that. A required `set_null` lookup
escalates to `restrict`. Either would stop an author deleting their own
comment once someone had reacted. So `comment_id` is an id column, like
`sys_comment.thread_id`. A deleted comment's reactions stay in storage
and are read by nobody, because the read gate fails closed on a comment
that no longer exists. This is pinned.
## Who grants the bits: the open decision
The card asks for "permission-set rows (create, read, delete-own)".
Measured on `origin/main` `25be87612d`:
- The shape mechanism hypothesis 2 points at does not exist.
`default-permission-sets.ts` `:1164`–`:1218` is the
`sys_comment_moderation` delete alternate and nothing else. The platform
ships no create or read bit on `sys_comment`. An app grants those:
hotcrm's members comment because objectstack-ai/hotcrm#2043 grants
`sys_comment` read, create and edit. The showcase and CRM examples grant
members nothing on `sys_comment`.
- The only shipped set every member resolves is `member_default`, the
`everyone` baseline. ADR-0090 D5 hard-blocks `allowDelete` there.
`describeHighPrivilegeBits` would answer "delete/purge/transfer on
'sys_comment_reaction'", `bindBaselineToEveryone` would then refuse to
bind the whole baseline, and `audience-anchor-set-claims.pin.test.ts`
pins `member_default` anchor-safe. So no shipped set can hand every
member a delete bit.
- The "delete-own" half needs no row: `owner_only_deletes` already does
it (pinned below).
Options, which the report on #22566 sets out on the four axes:
- **A.** `member_default` grants read and create on reactions
(anchor-safe), and delete stays app-granted. A member on an app that
grants no delete can add a reaction and never remove it.
- **B. This diff as it stands.** No platform grant: an app grants read,
create and delete on `sys_comment_reaction` wherever it grants
`sys_comment`, and the platform's floor keeps the delete to the
reactor's own row. A missing grant fails loudly on first use, for adding
and removing alike.
- **C.** Amend ADR-0090 D5 to admit an owner-scoped delete bit on an
`everyone`-bound set, then grant all three in `member_default`. This is
a Tier H ADR plus a `packages/spec` predicate change.
Recommended: B. The changeset states the B posture and says the baseline
grants nothing.
## Census of `sys_comment.reactions` consumers (report only, no edits)
- **objectstack** (`git grep reactions` over the tree at this head): the
field's declaration (`plugin-audit/src/objects/sys-comment.object.ts`)
and the four generated bundles. No runtime reader or writer.
`packages/spec/src/ui/component.zod.ts` `enableReactions`
(`RecordActivityProps`) is a feed display toggle that reads no column,
so it is a different surface and stays out.
- **objectui** `main` `d758f2f20d`: one reader and one writer, both
`packages/app-shell/src/views/RecordDetailView.tsx`. The read is
`dataSource.find('sys_comment', …)` at `:1907`, parsing `c.reactions` at
`:1920`. The write is `update('sys_comment', id, { reactions:
JSON.stringify(…) })` at `:2254`–`:2255`. Downstream renderers take the
parsed value, never the column: `plugin-detail`
`RecordActivityTimeline.tsx` and `ReactionPicker.tsx`
(`FeedItem.reactions`, `types/src/views.ts:650`).
`collaboration/src/CommentThread.tsx:25` takes a `reactions` prop, and
no in-repo caller feeds it. At the pinned `.objectui-sha` `47b1f0bb71`
the same single site appears at `:1884`, `:1897` and `:2226`–`:2227`.
- **cloud**: NOT MEASURED (no access from this session). The director's
reading (`6091621568`) is that cloud has no reader.
- This confirms the director's finding: objectui's chatter is the only
reader and the only writer. #22573 reads this.
## Tests
- `plugin-audit/src/comment-reaction-access.integration.test.ts` (real
kernel, real `ObjectQLPlugin`, real SQLite driver, `AuditPlugin`). For
every caller, the comments its readable reactions point at equal the
reacted comments it can read. It also pins `find`, `count`, `findOne`
and `aggregate`; the reader's one batched `comment_id` `$in` read, which
makes one comment probe per read and never one per row; the create
refusals on an unreadable comment, a missing comment, an empty
`comment_id` and a session with no user (`code` plus `status`);
`user_id` and `created_by` stamped with the reactor; two concurrent
reactions kept as two rows; the duplicate refused as 409 and mapped to
`UNIQUE_VIOLATION`; and a comment's author deleting the comment with
others' reactions on it.
- `plugin-audit/src/objects/sys-comment-reaction.object.test.ts` holds
the declaration to what the other rules read: no `sharingModel`, audit
columns on, `comment_id` with no reference or delete behaviour, the
index and its scope, `apiMethods` without `update`, and no
managed-affordance conflict.
- `plugin-security/src/comment-reaction-own-record-floor.test.ts` runs
the shipped `member_default` and a real `SecurityPlugin` over a real
engine and driver. It pins that a member creates a reaction on another
member's comment and deletes it; that another member cannot delete it,
by id (`PERMISSION_DENIED` 403) or by predicate, and the row survives;
and that two concurrent reactions each delete only by their reactor.
- `plugin-security/src/objects/default-permission-sets.test.ts`: no
shipped set carries a row-level policy on `sys_comment_reaction` (a
tripwire against a moderation-style alternate).
## Ablations (committed heads `8361ea8d82` and again `32c30d8034`, each
through `scripts/ablation-replace.mjs`, disk-verified, restored blob
equal to HEAD)
The subjects resolve from source (`plugin-security`'s own `src`, and
`plugin-audit`'s own `src` via `./audit-plugin.js`), so no `dist/` sits
on the path and no rebuild leg applies.
| leg | mutation | result |
|---|---|---|
| A1 | `owner_only_deletes` object `'*'` changed to an object that does
not exist (the own-record floor dropped) | floor test: 2 red (another
member's delete answers ok), 1 green |
| A2 | the reaction read middleware's registration deleted | integration
test: 9 red (every read pin, including the deleted-comment case), 12
green |
| A3 | the reaction insert hook's registration deleted (run at
`8361ea8d82` only) | integration test: setup red (`VALIDATION_FAILED`,
`user_id` required, since the stamp is gone), 21 skipped |
| A3b | the comment-readability check short-circuited, stamping kept |
integration test: 3 red (both refusal pins, and OTHER's count 4 not 3),
18 green |
| A4 | the app grant's `allowDelete` set to false | floor test: 2 red
(the member's own delete refused), 1 green |
On the card's ablation, "drop the own-record row and the first pin goes
red": the own-record row is the platform floor, and dropping it (A1)
reddens "cannot delete another member's reaction", not the first pin.
The first pin rests on the object grant (A4), and the grant's home is
the open decision.
## Gates
Local run at head `32c30d8034`. Exits are captured before any pipe.
- `@objectstack/plugin-audit`: `vitest run` gave 44 files and 701 tests
passed (at `cdd0198080`). The two reaction test files were re-run at
`32c30d8034` after their options were typed: 21/21 and 8/8. `typecheck`
exit 0, with the test layer compiled under `tsconfig.test.json`; both
new test files are in that program (`--listFilesOnly`).
- `@objectstack/plugin-security`: `vitest run` gave 194 files, 4079
passed and 45 skipped (at `1c5d95aa8c`; only the two test files below
changed after that). The floor test and
`default-permission-sets.test.ts` were re-run at `32c30d8034`: 31/31.
`typecheck` exit 0, with the test layer compiled.
- The gate list was derived with `node scripts/pm/dispatch-gates.mjs
--commands --repo objectstack-ai/objectstack`, with no paths given, from
the merge base `99801d831f`. 76 families were derived, 76 run and 76
exited 0. Reconciled with `--ran` (each command recorded with its exit):
"76 run, 0 NOT-MEASURED (a DERIVED zero)". The census gates' verdict
lines: `check:platform-object-tenancy-census` reports 84
platform-namespace objects, 49 in reach; `check:system-context-census`
reports 123 elevation read sites, with the page holding 118 of 118
symbols; `check:tenant-audit-census` is OK; `check:i18n` reports 9
packages in sync.
- NOT MEASURED: `check:i18n-coverage` (not derived for this diff). Its
prerequisite closure includes the four example apps, which were not
built here, so it is left to CI.
## Acceptance notes
- `sys_audit_log` still records every reaction create and delete: the
ledger capture is always on, and `activities: false` stops only the
`sys_activity` mirror. If the ledger's volume matters, the
`SKIP_OBJECTS` set in `audit-writers.ts` is where to change it. That
file is not touched here, because PR #22526 holds it.
- A deleted comment's reactions are not removed from storage, and nobody
reads them. No cleanup is built, because nothing has named a need for
one.
- `comment-access-hooks.ts`'s first header paragraph still says the
default member sets "grant wildcard CRUD"; `member_default` has had no
wildcard since #5491. This is stale prose, left as it was.
- The `isSystem` census page
(`content/docs/permissions/system-context.mdx`, row 45) describes the
comment hooks without naming reactions. Its anchors and counts still
hold, and the prose is a docs-only follow-up.
- REST door `PATCH` on the object is NOT MEASURED. The declaration is
pinned (`apiMethods` without `update`), and the door's `405` comes from
the existing `apiMethods` machinery, not from this diff.
- For objectstack-ai/objectui#12078: create with `comment_id` and
`emoji` (`user_id` is stamped and may be omitted). Read with one `find`
on `comment_id` `$in` the feed's ids. Remove by deleting the caller's
own row by id. A repeated reaction answers 409 `UNIQUE_VIOLATION`.
- The PR owes a contract-review-tier record before the queue (Clause-②
yes); the PM arranges it.
## Seat’s append: the open decision is answered (B)
Appended by `domain:services` seat 1
(`session_013j5gkUCpqQiti4GgPqqmnt`) at 2026-10-10T04:01Z. The seat
answered the dev’s open question in-seat as **B**, on #22566 (the answer
comment there): the governing texts already settle it. The maintainer’s
ruling recorded on `member_default` (#5491) says object access is the
application’s to declare, and ADR-0090 D5 hard-blocks `allowDelete` on
`everyone`-bound sets. So no code changes, and line 1 is rewritten from
`Part of #22566` to `Fixes #22566` by the seat. Where the sections above
call the bits’ home “the open decision”, read it as decided: application
permission sets, beside their `sys_comment` grant, with
`owner_only_deletes` keeping a delete to the reactor. #22500 remains
open; it also waits on objectstack-ai/objectui#12078.
## Seat's append: patch round 1 (head `9f26b002a1`)
Appended by `domain:services` seat 1
(`session_013j5gkUCpqQiti4GgPqqmnt`) at 2026-10-10T05:27Z, from the
dev's round-1 report (`6094184898` on #22566). Where the sections above
describe the object's `apiMethods`, or report results at `32c30d8034`,
**this section supersedes them**.
### What this adds: the object (replaces the first bullet)
- **The object**
(`plugin-audit/src/objects/sys-comment-reaction.object.ts`, registered
beside `sys_comment` in `AuditPlugin.init` and provisioned with it).
Fields: `comment_id` (text, at most 255 characters), `emoji` (text, at
most 64), `user_id` (lookup to `sys_user`), `created_at`. `apiMethods:
['get', 'list', 'create', 'delete', 'bulk']`; `userActions: { edit:
false, import: false }`; `feeds: false`, `activities: false`. No
`sharingModel`, and the audit columns are injected, so `created_by`
exists and is stamped. `@objectstack/spec`'s platform object registry
(`PLATFORM_OBJECTS_BY_PACKAGE['plugin-audit']`) lists it.
### The batch doors (`bulk`)
The object grants `bulk`, as `api-methods-batch-conformance.test.ts`
requires of every whitelist that grants single-record writes. It opens
no path around the rules above:
- The bulk gate is `bulk ∧ derived(child)`: a batched child is admitted
only when it is allowed on its own. `update` is not offered, so
`updateMany` and an `upsert` child stay refused.
- `createMany` (`createManyData`) is one `engine.insert` of the rows,
and the partial-success batch (`insertManyData`) is `engine.insertMany`.
The engine runs `beforeInsert` once per row in both, so every row meets
the create gate. Each row is stamped with the session's user, whatever
it sent. A row on a comment the caller cannot read refuses the whole
batch with `403 RECORD_NOT_ACCESSIBLE`, and nothing is written. Pinned
in `comment-reaction-access.integration.test.ts` under "bulk create" (3
tests).
- `deleteMany` (`deleteManyData`) deletes one id at a time, by id, so
every row meets `owner_only_deletes`. The caller's own reactions are
deleted, and another member's fails on its own row (`PERMISSION_DENIED`)
and survives. Pinned in `comment-reaction-own-record-floor.test.ts`
under "the batch doors keep the own-record floor", driven through the
real `ObjectStackProtocolImplementation` over the real engine.
- A batched read is an ordinary read, and the read gate narrows it. The
reader's `comment_id` `$in` read is the pinned case.
- Ablations at `11e9afaac5`. A1, dropping the floor, reddens the
`deleteMany` pin. A3b, short-circuiting the comment-readability check,
reddens both bulk-create refusal pins.
### Round 1 (patch): files and results at `9f26b002a1`
The two CI reds at `32c30d8034` (Test Core 1/6,
`@objectstack/spec#test:repo`) were this PR's own, and both are fixed at
the producer:
- `api-methods-batch-conformance.test.ts`: the object now grants `bulk`
(see "The batch doors").
- `platform-object-names.test.ts`:
`PLATFORM_OBJECTS_BY_PACKAGE['plugin-audit']` now lists
`sys_comment_reaction`. This is the one `packages/spec` file, declared
by the seat's claim amendment `6093649317` under triage's cross-domain
exception. `check:generated` proves all 15 generated artifacts current,
so none was regenerated. The changeset now grades `@objectstack/spec`
`minor` for the new registry name.
Files (21, +1300 / −63 against merge base `d748ae80af`):
- `.changeset/22566-comment-reaction-record.md`
- `packages/plugins/plugin-audit/scripts/i18n-extract.config.ts`
- `packages/plugins/plugin-audit/src/audit-plugin.ts`,
`audit-plugin.test.ts`
- `packages/plugins/plugin-audit/src/comment-access-hooks.ts`
-
`packages/plugins/plugin-audit/src/comment-reaction-access.integration.test.ts`
- `packages/plugins/plugin-audit/src/objects/index.ts`,
`sys-comment-reaction.object.ts`, `sys-comment-reaction.object.test.ts`
-
`packages/plugins/plugin-audit/src/translations/bundle-ownership.test.ts`,
and the seven generated bundle files (`en` / `es-ES` / `ja-JP` / `zh-CN`
`.objects.generated.ts`, three `.source-hashes.generated.ts`)
-
`packages/plugins/plugin-security/src/comment-reaction-own-record-floor.test.ts`
-
`packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts`
- `packages/spec/src/system/constants/platform-object-names.ts`
- `scripts/platform-object-tenancy-census.json`
Results (under the verify lock, exits captured before any pipe):
- `@objectstack/spec`: the repo project gave 54 files and 915 tests
passed (at `e4e0c355ca`). Of those, the two files CI failed gave 15/15.
`check:generated`: 15 of 15 up to date at `9f26b002a1`.
- `@objectstack/plugin-audit`: 44 files and 704 tests passed.
`typecheck` exit 0, with the test layer compiled.
- `@objectstack/plugin-security`: 194 files, 4080 passed and 45 skipped.
`typecheck` exit 0, with the test layer compiled.
- 98 derived gate families exited 0 (`dispatch-gates --ran`: "98 run, 0
NOT-MEASURED, a derived zero"). `check:dual-build-cjs-loads` needed four
unrelated packages built first, and was then re-run green.
`check:i18n-coverage` is OK: 13 configs, none new.
- Ablations re-run at `11e9afaac5`: A1 3 red / 1 green, A2 9 red / 15
green, A3b 5 red / 19 green, A4 3 red / 1 green. Every leg was restored
to a blob equal to HEAD.
- CI at `9f26b002a1` (the seat's reading, latest run per check name): 32
success, 3 rostered skips.
---
_Generated by [Claude
Code](https://claude.ai/code/session_013j5gkUCpqQiti4GgPqqmnt)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e223152 commit 29a163f
21 files changed
Lines changed: 1300 additions & 63 deletions
File tree
- .changeset
- packages
- plugins
- plugin-audit
- scripts
- src
- objects
- translations
- plugin-security/src
- objects
- spec/src/system/constants
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
43 | 44 | | |
44 | 45 | | |
45 | 46 | | |
| |||
60 | 61 | | |
61 | 62 | | |
62 | 63 | | |
63 | | - | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
147 | 147 | | |
148 | 148 | | |
149 | 149 | | |
| 150 | + | |
150 | 151 | | |
151 | 152 | | |
152 | 153 | | |
| |||
159 | 160 | | |
160 | 161 | | |
161 | 162 | | |
| 163 | + | |
162 | 164 | | |
163 | 165 | | |
164 | 166 | | |
| |||
176 | 178 | | |
177 | 179 | | |
178 | 180 | | |
| 181 | + | |
179 | 182 | | |
180 | 183 | | |
181 | 184 | | |
| |||
196 | 199 | | |
197 | 200 | | |
198 | 201 | | |
| 202 | + | |
199 | 203 | | |
200 | 204 | | |
201 | 205 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
| 6 | + | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
144 | | - | |
| 144 | + | |
| 145 | + | |
145 | 146 | | |
146 | 147 | | |
147 | 148 | | |
| |||
180 | 181 | | |
181 | 182 | | |
182 | 183 | | |
183 | | - | |
| 184 | + | |
184 | 185 | | |
185 | 186 | | |
186 | 187 | | |
| |||
349 | 350 | | |
350 | 351 | | |
351 | 352 | | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
352 | 356 | | |
353 | 357 | | |
354 | 358 | | |
| |||
388 | 392 | | |
389 | 393 | | |
390 | 394 | | |
391 | | - | |
392 | | - | |
| 395 | + | |
| 396 | + | |
393 | 397 | | |
394 | 398 | | |
395 | | - | |
| 399 | + | |
396 | 400 | | |
397 | 401 | | |
398 | 402 | | |
| |||
515 | 519 | | |
516 | 520 | | |
517 | 521 | | |
518 | | - | |
| 522 | + | |
| 523 | + | |
519 | 524 | | |
520 | 525 | | |
521 | 526 | | |
| |||
533 | 538 | | |
534 | 539 | | |
535 | 540 | | |
536 | | - | |
| 541 | + | |
537 | 542 | | |
538 | 543 | | |
539 | 544 | | |
| |||
0 commit comments