feat(plugin-audit): sys_comment gains a writer-stamped object_name + record_id pair with a backfill, and the comment-thread read gate leafs over it (ADR-0139 E7)

.changeset/23857-comment-parent-pair.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/plugin-audit': minor
+'@objectstack/cli': minor
+---
+
+feat(plugin-audit): `sys_comment` rows carry a writer-stamped parent pair, `object_name` + `record_id`, and the comment-thread read gate narrows by it with one `ReadableIds` leaf per parent object, with no pre-scan (ADR-0139 E7)
+
+**Run `os migrate comment-parent-pair --apply` at once after upgrading.** After the upgrade, and until it runs, the `sys_comment` rows written before the upgrade carry no pair, and the read gate answers them as it answered every comment before: by a SYSTEM pre-scan of at most 2,000 candidate rows per read, fail closed. So a broad comment list — one not scoped to a single record's thread — can omit comments the reader may see, and its `total` agrees with the short list; a record's own comment thread is unaffected. The door is `os migrate comment-parent-pair`, dry run by default, `--apply` to write, idempotent, and it stamps both columns in one run, keeping each row's `updated_at`. Its flags are `os migrate activity-changed-fields`'s, one for one (`--database-url`, `--apply`, `--yes`, `--force`, `--json`). The columns arrive with the upgraded deployment's schema sync, which an `--apply` run performs itself; a dry run against a database no upgraded process has opened cannot read them and exits `1`.
+
+Clause-②: yes
+
+`sys_comment` gains two `readonly` text columns and an index over them. They are derived from `thread_id` (`{object}:{record_id}`, split on the first colon) by one planner, `planCommentParentPair`, and `thread_id` stays the key authors write: nothing changes for a writer. A `sys_comment` before-insert hook stamps every inserted row, whoever writes it, over whatever value the writer sent; an update that names `thread_id` (a re-point) is re-stamped from it; and an update that names either column without a thread has the pair re-derived from the row's stored thread, for every caller — a historical import (`preserveAudit`) included, where the engine's update-side `readonly` strip keeps a value — so no writer's value for either column lands. A multi-row update that names either column without a thread is refused whole (`VALIDATION_ERROR` / 400, nothing written) when its rows' threads name different records, since one write cannot store each row's own pair; over rows sharing one thread it stores that thread's pair. A thread that names no record stamps `null` / `null`, and such a comment stays hidden from every non-system reader, as before.
+
+The comment-thread read gate (`installCommentReadVisibility`) is now the parent-record gate the activity stream and the compliance ledger use, over the comment's own pair: one branch `{ object_name: P, record_id: { $in: ReadableIds.of(P) } }` per registered object other than `sys_comment`, ORed and ANDed into the read's `where`, on `find`, `findOne`, `count` and `aggregate`. A broad comment read is served every comment the reader may read, on any table size, with a true `total`. What keeps the bounded 2,000-row fail-closed probe, chosen up front by name, never by catching a refusal:
+
+- ADR-0139 D5's three classes: a delegated (on-behalf-of) caller (the whole read); a parent narrowed by an object-scoped read middleware (`sys_attachment`, `sys_comment_reaction`, `sys_approval_request`, …); a parent bound to another datasource;
+- a security service that answers no `canReadObject`, or none at all (no leaf is written without it), and an engine that cannot classify a parent — each sends the whole read to the probe and is said once;
+- a row with no stamped pair (until the backfill reaches it): probed by its `thread_id`, restricted to rows with no pair, never leafed;
+- a composition whose registered `sys_comment` declares no pair: the thread probe over every row, said once.
+
+**The cost:** a deployment whose driver compiles no `ReadableIds` leaf — `driver-mongodb`, until ADR-0139's O3 — now fails a non-system comment read with `INTERNAL_ERROR` / 500 naming the driver in the log, as the activity, ledger and attachment reads already do (ADR-0139 D3); the gate does not catch that refusal into an empty list.
+
+Comment reactions are unchanged: their only parent is `sys_comment`, which this gate narrows by an object-scoped middleware, so the reaction gate keeps the bounded probe (ADR-0139 D5.2).
+
+`installCommentReadVisibility` takes an optional third argument, the security source whose `canReadObject` admits a leaf (`ParentObjectReadAdmissionSource`); without it every comment read is the bounded probe, as before. Its first parameter widens from `CommentAccessEngine` to `ParentRecordGateEngine` (`CommentAccessEngine` plus three optional members: a registry view, `hasObjectMiddleware` and `resolveEffectiveDatasource`); every existing caller's engine still satisfies it. Both types reach the published declaration through that signature. `CommentAccessEngine` gains an optional `getSchema`. `@objectstack/plugin-audit` exports the backfill, `backfillCommentParentPair` (with `COMMENT_PARENT_PAIR_BACKFILL_PAGE_SIZE` and its report and engine types), the planner `planCommentParentPair` (with `COMMENT_PARENT_OBJECT_COLUMN`, `COMMENT_PARENT_RECORD_COLUMN` and the `CommentParentPair` type).

content/docs/deployment/cli.mdx

@@ -1194,6 +1194,7 @@ where the data lives.
 | `os migrate comment-reactions` | Carry the emoji reactions a 17.x deployment stored in the retired `sys_comment.reactions` column into `sys_comment_reaction` records, list every stored value that does not read, and record the verified run that lets `os migrate apply --allow-destructive` drop the column |
 | `os migrate organization-ownership` | The v18 ownership ceremony (ADR-0131 D10): write a plan of what each table's rows will become, then `--apply` that reviewed plan. A v18 server refuses to start until it has run. See [below](#os-migrate-organization-ownership) |
 | `os migrate activity-changed-fields` | Stamp the two read-rule columns (`changed_fields`, `milestone_key`) on the `sys_activity` rows written before they existed. Until it runs, those rows are withheld from every reader but the system, so run it at once after upgrading. No flag; idempotent |
+| `os migrate comment-parent-pair` | Stamp the parent pair (`object_name`, `record_id`) on the `sys_comment` rows written before it existed. Until it runs, a broad comment read past 2,000 candidate rows omits comments the reader may see, and its total agrees with the short list, so run it at once after upgrading. No flag; idempotent |
 | `os migrate meta --stored` | Replay the metadata conversion chain over this deployment's `sys_metadata` rows and rewrite the ones still carrying a pre-protocol shape. Hygiene, not a gate — nothing depends on it having run |
 | `os migrate security-catalog-overlays` | List the environment-wide permission-set and position rows (the legacy plurals `permissions` / `positions` included) stored over a name a configured package holds — the rows a v18 cold boot refuses — each with the package that holds its name; `--apply` deletes them, one history tombstone per row. Run it before the first v18 boot, with the flags and environment the deployment boots with: the platform security plugin's sets are held only where `os serve` composes it (an auth secret set, and the `auth` tier on), and the step takes `serve`'s `--preset` and `--dev` with `serve`'s meaning. Adopts nothing; exits 1 while it lists a row |
 | `os migrate duplicates` | Report business identifiers already minted twice across the organization partitions, and the rows blocking the boot-time NULL-safe index tightenings — a read-only inventory as JSON on stdout. Renumbers nothing and writes no row and no schema; run it before the boot-time tenancy repair, which overwrites part of the evidence |
@@ -1499,6 +1500,36 @@ os migrate organization-ownership --dev …                    # boot as `os ser
   the security and audit plugins), the development `.env*` files, and the
   `telemetry` sibling. Pass it for a database that `os dev` serves.
 
+#### `os migrate comment-parent-pair`
+
+A comment is served to a reader only when the reader can see the record its
+thread names. Since this version that check reads two columns the platform
+stamps on every new comment from its `thread_id`, `object_name` and
+`record_id`, and narrows a read in one statement, on any number of comments.
+Rows written before the upgrade carry no pair, and the platform answers them
+the way it answered every comment before: it examines at most **2,000
+candidate rows per read** and hides the rest (it never shows a comment the
+reader may not see). So until this command runs, a broad comment list — one
+not scoped to a single record — can omit comments the reader may see, and its
+total agrees with the short list. A record's own comment thread is unaffected.
+
+```bash
+os migrate comment-parent-pair                    # Dry run: counts, writes no rows
+os migrate comment-parent-pair --apply            # Stamp the rows (prompts)
+os migrate comment-parent-pair --apply --yes --json   # CI / scripts
+```
+
+Each row is stamped from its own `thread_id`, by the rule the platform stamps
+new comments with; the row's `updated_at` is kept. **A comment whose thread
+names no record stays unstamped**, and stays hidden from every reader but the
+system, as before. The report counts those rows.
+
+**Idempotent**: only unstamped rows are read, so a second run stamps nothing,
+and re-running is how an interrupted run is finished. The columns arrive with
+the upgraded deployment's schema sync, which an `--apply` run performs itself:
+a dry run against a database no upgraded process has opened cannot read them,
+reports the failed read and exits `1`. A failed read or write exits `1`.
+
 #### A database created by this version needs no migration
 
 A deployment whose database the platform **creates from empty** records these

content/docs/permissions/tenant-audit-census.mdx

@@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way.
 
 The same holds twice over for the context. An options argument spelled as a
 literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
-forwarding shim cannot, and **53 of the 226 sites are spelled that way**. A
+forwarding shim cannot, and **53 of the 227 sites are spelled that way**. A
 context resolved from an inline literal or a local `const` can be tested for
 `isSystem`; one arriving from a helper call cannot.
 
@@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page:
 
 | carried figure | where it survives | this census |
 | :--- | :--- | ---: |
-| 175 write call sites | quoted in the merged changeset | **226** |
+| 175 write call sites | quoted in the merged changeset | **227** |
 | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **32** more whose options argument is unreadable |
-| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **155 of 226** decidable, **71** undecidable |
-| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 122 decidably elevated, 0 decidably not, 96 undecidable |
+| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **156 of 227** decidable, **71** undecidable |
+| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 123 decidably elevated, 0 decidably not, 96 undecidable |
 | 141 and 132, two independent re-derivations | the card that filed this work | — |
 
 **The differences are not reconciled, and deliberately so.** The old census's
@@ -201,17 +201,17 @@ at any commit.
 
 Two structural facts do plausibly widen this reading against any hand or regex
 one, and both are counted in the generated tables below: the 38 sites reached
-through an erased (`any`) receiver, and the 51 that name their object through a
+through an erased (`any`) receiver, and the 52 that name their object through a
 `const` rather than inline. An instrument that read either the way a person does
 would report a smaller number and would not say so.
 
 The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
 figure has no surviving corroboration anywhere in the tree.** This census reads
-122 of 226 (54%) as decidably elevated, with 96 more whose elevation is a
+123 of 227 (54%) as decidably elevated, with 96 more whose elevation is a
 run-time fact — so the claim is neither confirmed nor refuted, and the honest
 answer is that a static reading cannot settle it.
 
-⇒ **Cite `2 / 226`, and say what it is**: the sites whose options argument was
+⇒ **Cite `2 / 227`, and say what it is**: the sites whose options argument was
 READ and holds no tenant context, against a decidably tenancy-enabled object.
 That is the control's provable yield surface. ⛔ Do not cite it as "the sites
 without tenant context" — **32 further sites** have an options argument this
@@ -223,29 +223,29 @@ cannot read, and they are neither in nor out.
 
 | what | count |
 | :--- | ---: |
-| write call sites on the application surface | **226** |
-| …whose object name is statically decidable | 155 |
+| write call sites on the application surface | **227** |
+| …whose object name is statically decidable | 156 |
 | …whose object name is chosen at run time | 71 |
-| …against an object with tenancy ENABLED | 154 |
+| …against an object with tenancy ENABLED | 155 |
 | …against an object that declares tenancy off | 1 |
-| threading a tenant context | 165 |
+| threading a tenant context | 166 |
 | PROVABLY carrying none (options read, no context key) | **8** |
 | …of those, against a decidably tenancy-enabled object | **2** |
 | options argument UNREADABLE — may or may not carry one | 53 |
 | …of those, against a decidably tenancy-enabled object | 32 |
-| threading a decidably ELEVATED (`isSystem`) context | 122 |
+| threading a decidably ELEVATED (`isSystem`) context | 123 |
 | threading a context that is decidably NOT elevated | 0 |
 | threading a context whose elevation is a run-time fact | 96 |
 
 | how the instrument reached the site | count |
 | :--- | ---: |
-| receiver carried a readable engine type | 188 |
+| receiver carried a readable engine type | 189 |
 | receiver erased, placed by the object NAME | 28 |
 | receiver erased, placed by an `object: string` PARAMETER | 5 |
 | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 |
 
 | object name spelled inline | 104 |
-| object name spelled through a `const` | 51 |
+| object name spelled through a `const` | 52 |
 | object name is an `object: string` parameter | 7 |
 | object name is some other run-time expression | 64 |
 
@@ -297,12 +297,12 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-10-16 at `c53c594e7`.
+Measured on 2026-10-16 at `caebc6a7b`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 654 |
-| engine-shaped types recognised | 77 |
+| tracked non-test sources scanned | 655 |
+| engine-shaped types recognised | 78 |
 | declared objects in the registry | 116 |
 | same-named calls subtracted as non-engine | 165 |
 

docs/audits/2026-08-tenant-audit-write-call-sites.counts.md

@@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.
 
 | Measure | Value |
 |---|---:|
-| Write call sites | 226 |
-| Object name statically decidable | 155 |
+| Write call sites | 227 |
+| Object name statically decidable | 156 |
 | Object name chosen at run time | 71 |
-| Against a tenancy-enabled object | 154 |
+| Against a tenancy-enabled object | 155 |
 | Against an object declaring tenancy off | 1 |
-| Threading a tenant context | 165 |
+| Threading a tenant context | 166 |
 | Provably carrying none | 8 |
 | …and decidably tenancy-enabled | 2 |
 | Options argument unreadable | 53 |
 | …and decidably tenancy-enabled | 32 |
-| Threading a decidably elevated context | 122 |
+| Threading a decidably elevated context | 123 |
 | Threading a decidably non-elevated context | 0 |
 | Threading a context of undecidable elevation | 96 |
 
@@ -90,12 +90,12 @@ holds still. They are required to be HERE and to say WHEN they were true;
 their values are not compared. The reasoning, and the measurement behind it,
 are in `scripts/check-tenant-audit-census.mjs`.
 
-Measured on 2026-10-16 at `c53c594e7`.
+Measured on 2026-10-16 at `caebc6a7b`.
 
 | corpus scale (not enforced) | count |
 | :--- | ---: |
-| tracked non-test sources scanned | 654 |
-| engine-shaped types recognised | 77 |
+| tracked non-test sources scanned | 655 |
+| engine-shaped types recognised | 78 |
 | declared objects in the registry | 116 |
 | same-named calls subtracted as non-engine | 165 |
 
@@ -117,6 +117,7 @@ Measured on 2026-10-16 at `c53c594e7`.
 | `packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` | `update` | `objectPlan.object` | undecidable | context, elevation undecidable | 1 |
 | `packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts` | `update` | `sys_activity` | enabled | elevated | 1 |
 | `packages/plugins/plugin-audit/src/auth-event-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 |
+| `packages/plugins/plugin-audit/src/comment-parent-pair-backfill.ts` | `update` | `sys_comment` | enabled | elevated | 1 |
 | `packages/plugins/plugin-audit/src/read-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 |
 | `packages/plugins/plugin-audit/src/stored-metadata-body-migration.ts` | `update` | `object` | undecidable | elevated | 1 |
 | `packages/plugins/plugin-auth/src/admin-import-users.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 |