Skip to content

feat(plugin-audit): sys_comment_reaction, the reactor's own record of an emoji reaction - #22603

Merged
objectstack-fleet[bot] merged 14 commits into
mainfrom
claude/issue-22566-comment-reaction-record
Oct 10, 2026
Merged

objectstack-fleet[bot] merged 14 commits into
mainfrom
claude/issue-22566-comment-reaction-record

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

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_ids 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:

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. Retire sys_comment.reactions (ruling A amended on #22505): no aggregate, no data migration, after the console reads reaction records #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

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

… an emoji reaction

A reaction is one (comment, emoji, user) row owned by the reactor. The
comment gate's two installers answer for it with the same handler: a
reaction is readable exactly when its comment is (one caller-scoped read
of sys_comment), and created only on a comment the caller can read, with
user_id stamped from the session. Delete is the platform's own-record
floor; update is not offered.

Claude-Session: https://claude.ai/code/session_013j5gkUCpqQiti4GgPqqmnt
Co-authored-by: Claude <[email protected]>
…y census

Regenerated with node scripts/platform-object-tenancy-census.mjs --write.

Claude-Session: https://claude.ai/code/session_013j5gkUCpqQiti4GgPqqmnt
Co-authored-by: Claude <[email protected]>
node scripts/check-i18n-bundles.mjs --write --filter=audit

Claude-Session: https://claude.ai/code/session_013j5gkUCpqQiti4GgPqqmnt
Co-authored-by: Claude <[email protected]>
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-audit, @objectstack/spec, touching 31 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-audit/src/objects/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

62 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json d748ae80afe6ed63d1c61a51a69f4fb18280c844.

⛔ 13 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-audit/src/objects/index.ts) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: isSystem (5 routes)
  • 1 anchor(s) matched too much of the corpus to be a work list: sys_user (literal, 39 pages)
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 141 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d748ae80afe6ed63d1c61a51a69f4fb18280c844 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8efaab6e9a65315efd9b415737c26022805f8993 — the merge of head 9f26b002a1dc1225c27b15b3472ad678dcfe3609 into base d748ae80afe6ed63d1c61a51a69f4fb18280c844, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8efaab6e9a65315efd9b415737c26022805f8993 && git checkout 8efaab6e9a65315efd9b415737c26022805f8993
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d748ae80afe6ed63d1c61a51a69f4fb18280c844 9f26b002a1dc1225c27b15b3472ad678dcfe3609 && git checkout -B drift-repro d748ae80afe6ed63d1c61a51a69f4fb18280c844 && git merge --no-ff 9f26b002a1dc1225c27b15b3472ad678dcfe3609

node scripts/docs-audit/affected-docs.mjs --json d748ae80afe6ed63d1c61a51a69f4fb18280c844

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d748ae80afe6ed63d1c61a51a69f4fb18280c844 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

CI red at 32c30d8034: this PR's own, patch round in progress · domain:services seat 1 · session_013j5gkUCpqQiti4GgPqqmnt · 2026-10-10T04:13Z

Test Core (1/6) fails in @objectstack/spec#test:repo on two repo-wide conformance tests that enumerate platform objects. Both are caused by this diff:

The dev's patch round pushes both fixes and re-runs the spec repo tests locally. No re-run is spent on this head.


Generated by Claude Code

…lugin-audit's platform object registry

A whitelist that grants create and delete grants bulk too (api-methods-batch-conformance);
the batch doors still admit only children allowed one at a time, so updateMany stays refused.
PLATFORM_OBJECTS_BY_PACKAGE['plugin-audit'] lists the object the package now registers.

Claude-Session: https://claude.ai/code/session_013j5gkUCpqQiti4GgPqqmnt
Co-authored-by: Claude <[email protected]>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 9f26b002a1dc1225c27b15b3472ad678dcfe3609
Local-runs: none

PR #22603 (branch claude/issue-22566-comment-reaction-record) for card #22566, read at the head above as the net diff against origin/main d748ae8 (the merge base is the base tip, so the three-dot diff is exact): 21 files, +1300 / −63. Rendered 2026-10-10T05:45Z by the contract-review subagent inside the owning seat's session, read-only. Inputs: the card body and all seven comments (triage 6092508551, claim 6092717444, round-0 report 6093527449, the seat's answer 6093553917, claim amendment 6093649317, correction 6093657666, round-1 report 6094184898); the PR body and file list; the diff; the head's check-runs. Nothing was built, run or re-run. Where a claim below is "verified", it is verified by reading the code the claim rests on at this head or at the base tip, named inline.

Check-runs on the head (latest run per name, read in this act): 35 names, 30 success, 5 skipped (Auto Label, Build Docs, Check PR Size, Console Pin Gate, Packed-tarball smoke — the path-filtered and opt-in ones). All seven required contexts are success: Lint & Repo Gates, TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL), Governed Surface Queue Guard. Check Changeset is success. The file list touches no governed surface; this record is owed by the claim's Clause-②: yes, not by a governed path.

① Derived judgments

Every accept-set or public-surface change the diff implies, each with the verdict on how the PR names it.

  1. A new platform object, sys_comment_reaction, enters the data door. AuditPlugin.init registers and provisions it beside sys_comment; apiMethods: get, list, create, delete, bulk; userActions: edit false, import false; feeds: false, activities: false. Widening. Right. The object-level create, read and delete bits come from an application set; the shipped sets grant nothing on it (default-permission-sets.ts is unchanged in source, and the new tripwire in default-permission-sets.test.ts pins that no shipped set carries a row policy on the object).
  2. Create rule. The beforeInsert handler, now registered on both objects, takes the reaction branch: it refuses a session with no user, stamps user_id from the session over whatever the client sent, and refuses a comment_id that resolveReadableParentIds does not return under the caller's own envelope (callerContext, operation-private keys stripped). The evaluator is the comment gate's existing one, so no second readability rule exists. Right. The refusal code is the gate's existing RECORD_NOT_ACCESSIBLE 403 (an ADR-0112 catalog member), the no-user case included — one condition, one code, no new vocabulary.
  3. Read rule. One middleware handler on both objects; for reactions a system-context pre-scan of the caller's own where (fields: comment_id, READ_SCAN_LIMIT 2000, fail-closed truncation with a warn), then ONE caller-scoped read of sys_comment through resolveReadableParentIds, ANDed in as comment_id $in the stored values the pre-scan saw. That comment read runs the caller's sys_comment grant, the tenant layer, row scope and the thread gate; any exception in it answers the deny-all sentinel. Right, and the same shape as computeThreadVisibilityFilter. Two things I checked beyond the PR text: (a) an application that grants reaction read without comment read yields zero reactions, not a leak — the comment probe throws, resolveReadableParentIds catches to an empty set, the gate emits deny-all; (b) the one new nuance, passing the caller's orderBy into the pre-scan, is sound — fillQueryAstDefaults has already parsed it into SortNode[] on the same object, assertOrderByIsMaterializable refuses only formula-field sorts and the object declares none, the REST door refuses dotted sorts before the engine, and the aggregate and count ASTs carry no orderBy at all (EngineAggregateOptions has no such key; the aggregate opCtx.ast is object, where, groupBy, aggregations, having), so an alias-named sort cannot reach the pre-scan.
  4. Delete rule — no new code; the shipped floor is the whole rule. member_default.owner_only_deletes (object: '*', created_by == current_user.id, positions org_member) binds because the object declares no sharingModel and keeps the injected audit columns; the floor test drives the real SecurityPlugin with the shipped sets plus an app grant, by id and by predicate, and through the real deleteManyData loop. Right. The boundary of that guarantee, as shipped and unchanged here: principals outside the floor's position domain (org admins and owners; everyone-only anchors) are governed by the application's delete bit alone — the same as every other object under this floor, and consistent with sys_comment_moderation staying sys_comment-only (the tripwire holds no reaction alternate).
  5. Update is closed at every external door, and the posture does not rest on the hook alone. No beforeUpdate is registered for reactions; what closes update is apiMethods — REST enforceApiAccess via apiAccessDenialFromEnable on every data route including /batch and the per-object bulk routes, and runtime/action-execution.ts honours the same declaration for flow data actions — with owner_only_writes on '*' as the row-level backstop and userActions.edit: false as the affordance. Outside REST, the protocol's updateData is reached only by the import runner (import is create ∧ update-derived, and userActions.import: false) and by flow actions. Right, and the acceptance note "REST door PATCH NOT MEASURED" is answered by reading that machinery rather than by a run.
  6. bulk. isApiOperationAllowed is bulk ∧ child and upsert is create ∧ update, so createMany, deleteMany and /batch create and delete are admitted while updateMany and upsert stay refused. The engine runs beforeInsert per row for both insert(array) and insertMany (pinned: a batch with one unreadable comment refuses whole with nothing written), and deleteManyData deletes per id (pinned: another member's row fails on its own with PERMISSION_DENIED and survives). Right.
  7. Uniqueness. (comment_id, emoji, user_id) with unique: 'organization' (ADR-0120 D1): the driver prepends the NULL-safe organization key part. A duplicate is DUPLICATE_RECORD 409 at the engine and UNIQUE_VIOLATION 409 through mapDataError, no new code. Right. I also sized the key, because it is four varchar parts under utf8mb4 and the driver documents the 3072-byte bound as per KEY, not per column: the server refuses such a key with ER_TOO_LONG_KEY, explainUnkeyableTextColumn recognises that code beside ER_BLOB_KEY_WITHOUT_LENGTH, and for a UNIQUE index the driver then carries the constraint on its SHA-256 shadow column — the route sys_setting's four-part org-scoped unique key already takes. Not measured live on this object (Temporal Conformance runs driver-sql's own suite), but it is the platform's existing answer, not a gap this diff opens.
  8. comment_id as an id column, not a lookup, so a comment author's delete is neither refused by restrict nor made to cascade under the author's authority over other members' rows; orphaned reactions stay stored and are read by nobody (pinned). Right.
  9. @objectstack/spec: PLATFORM_OBJECTS_BY_PACKAGE['plugin-audit'] gains the name, so PLATFORM_PROVIDED_OBJECT_NAMES and isPlatformProvidedObjectName widen. Consumers traced at this head: lint's validate-object-references rung ③, validate-nav-access and validate-translation-references (a stack naming the object now resolves instead of being flagged), and engine.resolvePrimaryDatasource (the object votes for its datasource once registered, beside the three it is provisioned with). All widening; no accept set narrows. The api-surface snapshot records exports, not values, so check:generated 15 of 15 current is the expected reading, and TypeScript Type Check is green. Right.
  10. plugin-audit's public index is unchanged. objects/index.ts and COMMENT_REACTION_OBJECT are not re-exported from src/index.ts, so the package's published change is behavioural (a new registered object; two installers now gating two objects), not a new export. That is why the plugin-security test re-declares the floor-relevant shape instead of importing it, with plugin-audit's object test pinning the real declaration — the arrangement ownership-floor-alternates.test.ts already uses for sys_comment. Right.
  11. Generated artefacts. The four .objects.generated.ts and three .source-hashes.generated.ts bundles and scripts/platform-object-tenancy-census.json (84 registered, 49 in reach, tenant field organization_id) are generator output; check:i18n, check:i18n-coverage and check:platform-object-tenancy-census ride inside the green Lint & Repo Gates. Right.
  12. Untouched, as the card and claim require: sys_comment.reactions and its bundles, the author-only update floor on sys_comment, every content/docs page, every objectui file. Confirmed on the file list and the diff.

② Semver level

  • .changeset/22566-comment-reaction-record.md: @objectstack/plugin-audit: minor, @objectstack/spec: minor. Matches what the diff publishes. plugin-audit ships a new object, two gates and its bundles (files: dist); spec ships one registry row that widens three lint acceptances. Nothing is removed or renamed, so no migration text and no ADR-0087 disposition is owed; Check Changeset is green on the head.
  • @objectstack/plugin-security is correctly absent: its two changed files are tests, and the package publishes dist only. scripts/, .changeset/ and plugin-audit/scripts/i18n-extract.config.ts publish nothing.
  • Clause-②: Clause-②: yes (widening) in the changeset body; Clause-②: yes on PR body line 2. The arm is right: the diff widens what the data door, the batch doors and the lint registry accept and narrows nothing; yes takes at least minor, and both entries are minor.
  • The body carries the two sentences an upgrading agent needs: the B posture ("Granting it … the everyone-bound baseline grants nothing on it") and that sys_comment.reactions is unchanged in this release.

③ Boundary flags

Every dev flag and every open_questions entry, answered or escalated:

  • Round-0 open_questions (where the object's bits come from). Answered in-seat as B (6093553917) and corrected (6093657666): A is not barred by the member_default ruling but is a baseline widening no seat adds on its own; C would reverse ADR-0090 D5 and needs an ADR; B is the no-widening default and is what the diff ships. Consistent with the diff (no shipped grant; the tripwire; the floor pinned under an app grant). The seat recorded the answer as open to the maintainer's veto in its round report, so the escalation is on file; a veto for A is two member_default lines plus pins as a follow-up, and nothing here forecloses it. Round-1 open_questions is empty.
  • Round-0 deviations. (1) Part of to Fixes #22566: PR line 1 now reads Fixes #22566, and the check-runs "Part-of PR must not also close its card" and "The card this PR closes must claim this branch" are both success. Answered. (2) default-permission-sets.ts unchanged in source: the measured consequence of B. Answered. (3) one handler per seam in comment-access-hooks.ts: read in the diff; the comment branch keeps its behaviour, the comment suites sit inside the green Test Core, and check:system-context-census is green. Answered. (4) two existing plugin-audit tests updated: in the diff, within the claim's test surface. Answered. (5) origin/main merged: the head is a merge of d748ae8, the current base tip. Answered. (6) the card's ablation mapped to A1 and A4: explained — A1 reddens the "cannot delete another member's reaction" pin, A4 reddens the first pin. Answered. (7) translated-locale leaves carry the generator's source fills: the claim said "regenerated, never by hand", the source-hash files record the provenance, check:i18n is green; a hand translation is an i18n follow-up, not this card. Answered.
  • Round-1 deviations. The worktree was recreated; check:dual-build-cjs-loads needed four unrelated packages built and was then re-run green; the spec repo project ran at an earlier branch commit with the two target files re-run at the changeset commit and check:generated at the head — all superseded by the head's own check-runs, which are the gate verdicts here; bulk is pinned at the engine calls and through ObjectStackProtocolImplementation rather than at the REST 405 — answered in ①.5 and ①.6 by reading the apiMethods machinery.
  • out_of_scope_findings (three, noted and not filed). Judged against Prime Directive 10: the stale "wildcard CRUD" sentence in comment-access-hooks.ts's header and the system-context.mdx row that does not yet name reactions are prose, not a defect, a contract violation or an authoring trap, so Acceptance notes is the right carrier; sys_audit_log recording reaction writes is the ledger's always-on design (activities: false stops only the sys_activity mirror), with SKIP_OBJECTS named as the knob and audit-writers.ts held by PR fix(plugin-audit): sys_activity.actor_name carries the acting user's display name, written with the row #22526 — a note, not a defect.
  • The card's measurements. Read rule: measured, the comment gate reused (①.3). Uniqueness: measured (①.7). Consumer census: objectstack has no runtime reader; objectui's RecordDetailView.tsx is the one reader and writer at main and at the pin; cloud NOT MEASURED (no access from the session) — escalated plainly, and harmless to this additive diff since nothing is retired. The pinned console's write to sys_comment.reactions is untouched, so no window opens where a member's click fails worse than today; that ordering belongs to Retire sys_comment.reactions (ruling A amended on #22505): no aggregate, no data migration, after the console reads reaction records #22573.
  • Triage's five pins are all pinned: own-reaction create and delete on another member's comment; two concurrent rows; another member cannot delete; readable exactly when the comment is; the ablation, with the dev's mapping stated.
  • Acceptance-note "REST door PATCH NOT MEASURED": answered by reading (①.5).

One observation, not a FAIL reason and not pinned either way: whether a reactor can still delete their own reaction by id once its comment is gone or no longer readable to them is not pinned; the orphan is read by nobody (pinned), and no ruling asks for a retraction path. If a chatter card needs one, it is a question for that card.

Implemented-by: claude/issue-22566-comment-reaction-record
Reviewed-by: session_013j5gkUCpqQiti4GgPqqmnt (contract-review subagent, CONTRACT_REVIEW_TIER)

VERDICT: PASS


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation protocol:system size/xl tests tooling

Projects

None yet

2 participants