Skip to content

Commit 8bd0fcd

Browse files
fix(plugin-audit): a fired milestone row carries metadata.kind 'milestone' (ADR-0052 §5), served to a reader served its watched fields (#22783)
Fixes #22771 Clause-②: no ## What changed - `packages/plugins/plugin-audit/src/audit-writers.ts`: the `activityMilestones` branch of the CRUD mirror writes `metadata.kind: 'milestone'` on the row it writes. `type` stays what the declaration names: `completed`, or `updated` when it names none. ADR-0052 §5 puts the kind in this slot: "`type` stays domain-NEUTRAL … Domain kind rides in `metadata.kind`." No new `type` value, no new column, no `packages/spec` change. - The same row records where the kind came from. `metadata.text_sources.kind` lists every field that a declared milestone watches. The first match wins, so "a milestone fired" is a fact about all of those fields, not only the one that fired. - `packages/plugins/plugin-audit/src/activity-field-redaction.ts`: `redactActivityRows` keeps `metadata.kind` for a reader served every listed field, and drops it for a reader who is not. A kind on a row the mirror did not write belongs to its author and is served as written. Text columns get the same treatment today. - A `@objectstack/plugin-audit` patch changeset. ## Measured on `origin/main` e84aeb3 before writing | PM assumption | Reading | |---|---| | 1a. The milestone branch sets `activityType = milestone.type` and records no provenance | **Holds.** `audit-writers.ts` has `if (milestone.type) activityType = milestone.type;`. The row carries `text_sources`, but only for the interpolated tokens and the label. Nothing marks it as a milestone. | | 1b. `metadata.text_sources` is written by the CRUD mirror only | **Holds, with one precision.** The milestone branch is part of the CRUD mirror (same `writeAudit`). So every mirror row carries `text_sources`, milestone or not, and it cannot tell them apart. | | 1c. `redactActivityRows` deletes `text_sources` before serving | **Holds.** | | The card's "the redaction keeps `metadata.kind`" needs a code change | **Falsified.** On main the redaction already serves any top-level `metadata` key other than `text_sources` untouched: it narrows only `old`/`new`. The "keep" half needed no code. The code added is the opposite direction (assumption 3). | | 2. No `sys_activity` writer already uses `metadata.kind`, and no `kind` vocabulary is declared | **Holds.** `git grep "metadata\.kind"` outside docs: 0 hits. The only hit is ADR-0052 line 201. Control on the same subject and failure shape: `metadata\.old` or `metadata\.new` in `packages/**/*.ts`: 8 hits. The repo's only `sys_activity` insert is `persistAuditTrailRow` (`audit-writers.ts`), called by the CRUD mirror alone. `auth-event-audit` and `read-audit` write `sys_audit_log` only. Comments write `sys_comment`. `packages/spec` declares `SYS_ACTIVITY_BUILTIN_TYPES` and `FeedItemType` for `type`, and nothing for `metadata.kind`. App writers in other repositories (hotcrm) cannot be grepped here, and their kinds stay served as written. | | 3. Keeping `metadata.kind` exposes a classification only, never field content | **Not unconditionally.** `kind: 'milestone'` means "a watched field just entered its declared value". To a reader not served that field, that is field content. So the kind is judged like a text: it is kept only for a reader served every watched field. | ## Keys the redaction keeps, before and after These are the top-level `metadata` keys a reader with a security answer is served on a CRUD-mirror row. Fail-closed paths are unchanged: they drop the whole `metadata` column. The no-answer path is unchanged: it passes the row whole. | key | before | after | |---|---|---| | `old` / `new` | kept; each key the reader is not served is dropped | unchanged | | `text_sources` | dropped, never served | unchanged | | `kind` (milestone rows only) | not written | kept when every field in `text_sources.kind` is served; dropped otherwise | | any other key, on a row an app wrote | served as written | unchanged | ## Tests (HEAD `f5281e88b`) New file: `src/activity-milestone-kind.integration.test.ts`, 10 tests. It uses a real `ObjectKernel`, the real SQLite driver and the real `AuditPlugin`. Rows are written by the real CRUD mirror and read through the middleware chain the plugin mounts: the console's activity read (`find` on `sys_activity` by `object_name` + `record_id`) and the by-id read, as non-system callers. The only stand-in is the security service. - A milestone row is served with `metadata.kind: 'milestone'`, through both the list read and the by-id read. This holds for a typed milestone (`completed`) and an untyped one (`updated`). - CONTROL: CRUD-mirror rows that no milestone fired (create, tracked update) carry no kind, at rest or served. - CONTROL, key for key: the whole served `metadata` of a milestone row is pinned per reader with `toEqual`. `text_sources` is never served, a withheld change key is dropped from both sides, and `kind` is the only key judged by its declared fields. - A reader withheld a watched field gets both milestone rows without the kind. This includes the untyped one, whose own field that reader is served. - A reader withheld a field that no milestone watches still gets the kind. - A kind on an app-written row is served as written to every reader. Runs (under `scripts/pm/os-verify-lock.sh`): - `pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=2`: Test Files 46 passed (46), Tests 726 passed (726), at `0655363f8`. The only later commit, `f5281e88b`, is a type annotation in the new test. The new file re-ran at `f5281e88b`: 10 passed. - `pnpm --filter @objectstack/plugin-audit typecheck`: VERDICT command-exit 0 at `f5281e88b`. `check:test-typecheck: OK … 0 file(s) / 0 error(s)` in debt. The first run had 2 TS2345 errors in the new test (`Row | null` passed into the metadata reader); `f5281e88b` fixed them. ## Ablations (committed state `2f5b81f3e`; each leg went through `scripts/ablation-replace.mjs` with its own restore) The suite resolves the subject through relative `src/` imports, not through a package `exports`, so no `dist/` rebuild is needed per leg. Each leg's anchor hit 1 time before and 0 times after, and the blob changed. Each restore was proven by blob equal to HEAD and an empty `git diff HEAD`. The tree was clean afterwards. | leg | mutation | result | |---|---|---| | writer-no-stamp | the milestone branch no longer sets the kind | 5 failed / 5 passed: at-rest kind, control served, note-withheld served, by-id, key-for-key | | redaction-deletes-kind (the PM's "put the deletion back") | the redaction deletes `kind` for every row | 5 failed / 5 passed: control served, note-withheld, by-id, key-for-key, app-written kind | | redaction-no-gate | the redaction never drops `kind` | 3 failed / 7 passed: watch-withheld reader, by-id, key-for-key | | writer-fired-only | `text_sources.kind` lists only the fired milestone's field | 2 failed / 8 passed: at-rest declared fields, and the watch-withheld reader on the untyped milestone | ## Gates (HEAD `f5281e88b`) `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 67 commands. All 67 were run, and each exit code was recorded before any pipe. - 65 exited 0 on the first pass. - `pnpm check:dual-build-cjs-loads` and `pnpm check:i18n` first exited 3 (PREREQUISITE NOT MET: no `dist/`). They exited 0 once `check:type-check-debt` had built the workspace (i18n: "OK (9 package(s) — all bundles in sync"). - `--ran` reconciliation: "67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN". Lint was narrowed, and the narrowing is measured. `pnpm exec eslint --no-inline-config --format json` on the 3 changed `.ts` files gave 3 files in the JSON, 0 errors and 0 warnings. These are the diff's whole TypeScript population; the fourth path is a `.changeset` Markdown file, which no `files` glob in `eslint.config.mjs` matches. `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`), so this diff cannot change the verdict on any file it does not touch. ## Acceptance notes - **Pre-existing, outside this card (for triage).** A reader withheld a milestone's watched field is still served that milestone row's `type`, which comes from the declaration, and its `summary`. The summary keeps the template's literal wording, and `text_sources.summary` names only the interpolated tokens, never the watched field. Probe at `2f5b81f3e`, through the same served read, as a reader withheld `stage`: `{type: 'completed', summary: 'Won item'}`, with `stage` redacted out of `metadata`. Both columns state that the withheld field entered `won`. This PR's `kind` is gated, so it adds nothing to that disclosure. The type and summary halves are reported for the seat to route. - The objectui#12106 reader that consumes the kind sees it only for a reader served every watched field. A reader who is not served one of them keeps today's behaviour, by design. - Rows written before this change carry no kind and are not backfilled. The 14-day activity retention ages them out. --- _Generated by [Claude Code](https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c1b7f2b commit 8bd0fcd

4 files changed

Lines changed: 398 additions & 13 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
---
4+
5+
A `sys_activity` row that an `activityMilestones` entry fired carries `metadata.kind: 'milestone'`, so a reader can tell a milestone from an ordinary update or a finished task
6+
7+
Clause-②: no
8+
9+
ADR-0052 §5 keeps `sys_activity.type` domain-neutral and puts a row's domain kind in
10+
`metadata.kind`. A fired milestone keeps the `type` its declaration names: `completed`, or
11+
`updated` when it names none. Nothing on the served row said it was a milestone. So a "Case
12+
resolved" milestone declared with `type: 'completed'` read as a finished task, and a feed that
13+
hides completed tasks hid it too.
14+
15+
The CRUD mirror now writes `metadata.kind: 'milestone'` on every row a milestone fires. `type` is
16+
unchanged, and a row that no milestone fired carries no `kind`.
17+
18+
The kind is served by the same rule as the text beside it. "A milestone fired" means that a
19+
watched field entered its declared value, so:
20+
21+
- **A reader served every field that a declared milestone watches** is served `metadata.kind`.
22+
- **A reader not served one of those fields** is served the row without `metadata.kind`, just as
23+
it is served the change without that field.
24+
- **A row an app writes itself** keeps its own `metadata.kind` for every reader, as written.
25+
26+
The rest of the redaction is unchanged. The provenance declaration (`metadata.text_sources`) is
27+
never served, and a change key the reader is not served is dropped. Rows written before this change
28+
carry no kind, and this change does not backfill them. Nothing you author changes: no key, option,
29+
export or type is added or removed.

‎packages/plugins/plugin-audit/src/activity-field-redaction.ts‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@
4848
* is kept. Dropped, not masked, for the precedent's reasons: the contract
4949
* publishes WHICH fields are masked, not the masked value, and dropping
5050
* discloses strictly less.
51+
* - `metadata.kind` ({@link ACTIVITY_KIND_KEY}, ADR-0052 §5): on a row the
52+
* mirror wrote, the kind is DERIVED from parent fields, so it is judged like
53+
* a text. The mirror stamps `'milestone'` when a declared `activityMilestones`
54+
* entry fired, which is the fact that a watched field entered its declared
55+
* value; the declaration names the watched fields, and a reader not served
56+
* one of them is served the row without the kind. A reader served them keeps
57+
* it: it is a classification, and every key of it is one the reader may
58+
* read anyway. A kind on a row someone else wrote is the author's, served as
59+
* written, like its text.
5160
* - The declaration itself is bookkeeping and is never served to a reader: it
5261
* names fields, and which field names a reader may see is the metadata
5362
* plane's question, not this one's.
@@ -142,10 +151,30 @@ const SYSTEM_CTX = { isSystem: true } as const;
142151
*/
143152
export const ACTIVITY_TEXT_SOURCES_KEY = 'text_sources';
144153

154+
/**
155+
* [#22771] The `metadata` key a row's DOMAIN kind rides in. ADR-0052 §5: "`type`
156+
* stays domain-NEUTRAL … Domain kind rides in `metadata.kind`." So a fired
157+
* milestone keeps the `type` its declaration names (`completed`, or `updated`
158+
* when it names none) and says what it is here, which is how a reader tells a
159+
* "Case resolved" milestone from a finished task. Written by `audit-writers.ts`,
160+
* judged here: one name, imported by both halves.
161+
*/
162+
export const ACTIVITY_KIND_KEY = 'kind';
163+
164+
/** [#22771] The kind the CRUD mirror stamps on a row a declared `activityMilestones` entry fired (ADR-0052 §5b.2). */
165+
export const ACTIVITY_MILESTONE_KIND = 'milestone';
166+
145167
/** The declaration's shape. */
146168
export interface ActivityTextSources {
147169
summary: string[];
148170
record_label: string[];
171+
/**
172+
* [#22771] The parent fields `metadata.kind` was derived from, present only
173+
* beside a kind the mirror stamped. For a milestone it is every field a
174+
* declared milestone watches, not only the one that fired: the kind says
175+
* that SOME milestone fired, which is a fact about all of them.
176+
*/
177+
kind?: string[];
149178
}
150179

151180
/** The text columns a declaration covers. */
@@ -197,6 +226,20 @@ function textProvenance(metadata: Record<string, unknown> | null): Record<TextCo
197226
return earlierMirrorShape ? { summary: 'unknown', record_label: 'unknown' } : null;
198227
}
199228

229+
/**
230+
* [#22771] Where a row's `metadata.kind` came from, read the way
231+
* {@link textProvenance} reads its text:
232+
* - a field list — the mirror's declaration;
233+
* - `'unknown'` — a mirror row whose declaration names no list for it;
234+
* - `null` — no kind, or a row the mirror did not write: served as written.
235+
*/
236+
function kindProvenance(metadata: Record<string, unknown> | null): string[] | 'unknown' | null {
237+
if (!metadata || !(ACTIVITY_KIND_KEY in metadata) || !(ACTIVITY_TEXT_SOURCES_KEY in metadata)) return null;
238+
const declared = metadata[ACTIVITY_TEXT_SOURCES_KEY];
239+
const list = isRecord(declared) ? declared[ACTIVITY_KIND_KEY] : undefined;
240+
return Array.isArray(list) && list.every((f) => typeof f === 'string') ? (list as string[]) : 'unknown';
241+
}
242+
200243
/** Drop every value-bearing column — the fail-closed answer for one row. */
201244
function stripValueBearing(row: Record<string, unknown>): void {
202245
for (const col of VALUE_BEARING_COLUMNS) delete row[col];
@@ -355,13 +398,21 @@ export async function redactActivityRows(
355398

356399
const metadata = 'metadata' in row ? parseMetadata(row.metadata) : null;
357400
const provenance = textProvenance(metadata);
401+
const kindSources = kindProvenance(metadata);
402+
/** Whether a value composed from `sources` is served to this reader. */
403+
const servedWhole = async (sources: string[] | 'unknown') =>
404+
sources === 'unknown' ? !(await restrictedOn(object, servedSet)) : sources.every((f) => servedSet.has(f));
358405

359406
if (metadata) {
360407
let changed = false;
361408
for (const side of ['old', 'new'] as const) {
362409
const snapshot = metadata[side];
363410
if (isRecord(snapshot) && dropUnservedKeys(snapshot, servedSet)) changed = true;
364411
}
412+
if (kindSources !== null && !(await servedWhole(kindSources))) {
413+
delete metadata[ACTIVITY_KIND_KEY];
414+
changed = true;
415+
}
365416
if (ACTIVITY_TEXT_SOURCES_KEY in metadata) {
366417
delete metadata[ACTIVITY_TEXT_SOURCES_KEY];
367418
changed = true;
@@ -372,12 +423,7 @@ export async function redactActivityRows(
372423
if (!provenance) continue;
373424
for (const col of TEXT_COLUMNS) {
374425
if (!(col in row)) continue;
375-
const sources = provenance[col];
376-
const servedWhole =
377-
sources === 'unknown'
378-
? !(await restrictedOn(object, servedSet))
379-
: sources.every((f) => servedSet.has(f));
380-
if (!servedWhole) delete row[col];
426+
if (!(await servedWhole(provenance[col]))) delete row[col];
381427
}
382428
}
383429
}

0 commit comments

Comments
 (0)