Skip to content

Commit e952cff

Browse files
fix(service-knowledge): drop the record.* branch from the realtime event bridge and name data.record.* in its docs (#21140)
Fixes #20573 Clause-②: no The knowledge plugin's realtime bridge still carried a `record.created` / `record.updated` / `record.deleted` branch (with a `payload.record ?? payload` fallback), and its docs named `record.*` as the ObjectQL events it syncs. No producer emits those names, in this repository or in cloud. The branch is removed and the docs now name `data.record.created|updated|deleted`. ## What changed - `knowledge-service-plugin.ts`: removed the `record.*` branch and the `payload.record ?? payload` fallback. The `[#4626]` comment no longer contrasts with a shape that does not exist; it states the three names ObjectQL publishes. The `enableEventSync` option TSDoc names `data.record.created|updated|deleted` and says the `data.records.*` aggregate events only warn. - `knowledge-service.ts`: `handleRecordUpsert` / `handleRecordDelete` docs name `data.record.*` (record body from `after`, id from `recordId`). - `event-sync-data-events.test.ts`: one new describe, four cases (below). - `.changeset/20573-knowledge-record-branch.md`: `patch` for `@objectstack/service-knowledge` (published package, no `private` flag). The `data.record.*` sync and the `data.records.*` stale-index warning are byte-for-byte unchanged. ## Premise check (measured at origin/main 7a606a9, the branch point) In-repo producers, three steps because the first instrument cannot answer the question: - `git grep -nE "publish\(" -- 'packages/**/*.ts' ':!**/*.test.ts'` returns 38 lines, and none of them carries an event name (the type is built in a variable or template). Filtering them for `record.` event names gives 0, but the same-subject control (a `data.record.` name on a `publish(` line) also gives 0, so that count is void. It is not used as evidence. - Widened instrument, realtime-publishing call sites: 3 (`metadata-manager.ts:723`, `engine.ts:7445`, `engine.ts:7542`). Control, same file: `engine.ts` builds `type: data.record.ACTION` (`:7428`) and `type: data.records.ACTION` (`:7527`), 16 `data.record(s).` lines in the file. `metadata-manager.ts` publishes `metadata.TYPE.ACTION`. - Bare `record.(created|updated|deleted)` as an event name in non-test source (`git grep -nP` with a not-preceded-by-`data.` lookbehind over `packages` and `examples`): 0 producers. The only hits naming it as an event are the branch removed here, its two doc comments changed here, and the spec's own retirement records (`realtime.zod.ts`, the realtime-event-type migration entry), which state that no producer ever emitted these names. Field-path uses such as `record.created_at` are not events. - Cross-repo: the `repo:cloud` seat read cloud `main` `72b8415a` (comment 5925992601 on the card): 1 hit for the bare names, a doc comment about the field `record.created_by`; control 35 hits. So cloud has no producer either. - Tests: no existing `service-knowledge` test fed a bare `record.*` event to the handler, so none pinned the removed branch and none was converted or deleted. Docs (`content/docs`) and `skills/` do not name the bare events. ## Tests `pnpm --filter @objectstack/service-knowledge test`: 4 files, 44 tests pass. The package typecheck passes, and `tsc --listFiles` shows the edited test file is compiled by both the package and the test tsconfig. New pin, four cases in `event-sync-data-events.test.ts`. Each delivers the bare shape on the subscription and asserts no `handleRecordUpsert` / `handleRecordDelete` call, then delivers the same row or id as `data.record.*` on the SAME subscription and asserts it IS synced (the control, so a dead harness cannot pass for a removed branch): - `record.created` with the row under `payload.record`; - `record.updated` with the row as the payload itself (the old `?? payload` fallback); - `record.deleted` with the id from `payload.id`; - `record.deleted` with the id from `payload.record.id`. Ablation, run from the committed fix (HEAD blob 495cad1): the original branch put back verbatim with `scripts/ablation-replace.mjs` in wrap mode. - Mutation proof from the tool: anchor 1 to 0, blob `495cad164104` to `6345444c7301`. - Result, red: `Tests 4 failed | 5 passed (9)`, failing with `expected "handleRecordUpsert" to not be called at all, but actually been called 1 times` (and the same for `handleRecordDelete`). The five pre-existing tests stayed green. - Restore proven by the tool: blob after restore equals the HEAD blob, `git diff HEAD` empty. - The first ablation leg was a no-op and is void: my replacement text re-contained the anchor, so the tool refused (anchor count 1 to 1) before the tests ran. It restored cleanly; the second leg above used an anchor the replacement does not reproduce. - The subject is the package's own source file imported by relative path, so vitest reads `src`, not a dependency's `dist`; the red above is the proof it read the mutated file. ## Gates Derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (no path arguments; the script takes the change set from the merge base, as the standing rule requires). 61 commands, each run in the foreground with its exit code recorded beside the verbatim command; `--ran` reconciles `61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN`. - 60 exit 0. - NOT MEASURED: `pnpm check:dual-build-cjs-loads` exits 3, `PREREQUISITE NOT MET`: it reads built `dist/` for 65 packages across the repo and this worktree builds only the dependency closure of one package. This diff touches no `exports`, build config or manifest. CI owns that scan. - `pnpm check:type-check-debt` first exited 124 (my own 280 second timeout under shared-box contention, not a verdict); rerun under the lock with a larger budget: `OK — 1 ledger entr(ies) re-measured in 103.7s, none above its recorded number`. `service-knowledge` has no ledger entry. - The derivation printed a stale-tree warning (origin/main had moved 8 commits, 3 files it reads changed: a `fleet-write.yml` action bump, one `engine-double-contract.pinned.json` row for another test file, the root `package.json`). I derived again in a throwaway detached worktree at origin/main `39ab2940` with this diff applied: the 61-command list is identical. That worktree is removed. - Lint, narrowed: `eslint --no-inline-config --format json` on the 3 changed TypeScript files, 3 files linted (the count is read from eslint's own JSON), 0 errors, 0 warnings. `eslint.config.mjs` never enables type-aware linting (no `parserOptions.project`, noted at `:328`), so this diff cannot move the verdict of any untouched file. The repo-wide `pnpm lint` is CI's. - Commits at final head `3e588a22`: the gates above ran at it. The whole-package test ran at `8cd6a922` (44 tests) and the pin file again at `3e588a22` (9 tests); the only change between those two commits is the changeset file. ## Acceptance notes Not filed, not fixed here: - Bare `record.created|updated|deleted` still appears as arbitrary event-name test data in `packages/services/service-realtime/src/in-memory-realtime-adapter.test.ts`, `packages/spec/src/contracts/realtime-service.test.ts`, and a few spec schema tests (`websocket.test.ts`, `protocol.test.ts`, `connector-author-shape.test.ts`). The adapter's event `type` is a plain string and nothing at runtime reads those names. No non-test source or doc advertises them. Carrier: none. - The subscription takes no `eventTypes` filter, so the handler sees every event, including `metadata.*`, and falls through on any name it does not know. That was already the behaviour for `metadata.*` and is unchanged. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b3917d9 commit e952cff

4 files changed

Lines changed: 99 additions & 29 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/service-knowledge': patch
3+
---
4+
5+
fix(service-knowledge): the realtime event bridge no longer keeps a `record.created` / `record.updated` / `record.deleted` branch, and its docs name the events ObjectQL really publishes (#20573)
6+
7+
No producer emits a bare `record.*` event: the ObjectQL engine publishes `data.record.created` / `data.record.updated` / `data.record.deleted` for a single-record write and `data.records.updated` / `data.records.deleted` for a predicate write (`multi: true`), and `@objectstack/spec` already dropped the bare names from `RealtimeEventType`. The `KnowledgeServicePlugin` subscription handler still carried a branch for them, with a `payload.record ?? payload` fallback for a shape nothing sends. That branch is removed. The `data.record.*` sync (record body from `after`, id from `recordId`) and the `data.records.*` stale-index warning are unchanged.
8+
9+
The `enableEventSync` option's TSDoc and the `handleRecordUpsert` / `handleRecordDelete` docs now name `data.record.created|updated|deleted` instead of `record.*`.
10+
11+
If a plugin of yours publishes a bare `record.created`, `record.updated` or `record.deleted` event through `IRealtimeService` and relied on the knowledge index following it, publish `data.record.created|updated|deleted` with the `DataEvent` payload instead (the record in `after`, its id in `recordId`).

‎packages/services/service-knowledge/src/__tests__/event-sync-data-events.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,66 @@ describe('#4626 — KnowledgeServicePlugin event sync on data.record.*', () => {
168168
expect(message).toContain('application-level predicate writes are not');
169169
});
170170
});
171+
172+
/**
173+
* No producer emits a bare `record.created|updated|deleted` event: the ObjectQL
174+
* engine publishes `data.record.*` (and `data.records.*` for a predicate
175+
* write), and the spec's `RealtimeEventType` no longer contains the bare names.
176+
* The bridge therefore has no branch for them — and no `payload.record ??
177+
* payload` fallback to read a record out of a shape nothing sends. Each case
178+
* delivers the legacy shape and then the same record as `data.record.*` on the
179+
* SAME subscription: the legacy delivery must sync nothing while the
180+
* `data.record.*` one does, so a silent harness cannot pass for a removed branch.
181+
*/
182+
describe('a bare record.* event is not part of the sync vocabulary', () => {
183+
const ROW = { id: 'task_1', title: 'Ship it', notes: 'the record body' };
184+
const at = '2026-08-02T12:00:00.000Z';
185+
const legacy = (type: string, payload: Record<string, unknown>): RealtimeEventPayload => ({
186+
type,
187+
object: 'task',
188+
payload,
189+
timestamp: at,
190+
});
191+
192+
it.each([
193+
['record.created', 'under payload.record', { record: ROW }],
194+
['record.updated', 'as the payload itself', ROW],
195+
])('%s (%s) upserts nothing, while data.record.created upserts the same row', async (type, _shape, payload) => {
196+
const harness = makeCtx();
197+
const { service, deliver } = await harness.boot(new KnowledgeServicePlugin());
198+
const upsert = vi.spyOn(service, 'handleRecordUpsert').mockResolvedValue(undefined);
199+
const del = vi.spyOn(service, 'handleRecordDelete').mockResolvedValue(undefined);
200+
201+
await deliver(legacy(type, payload));
202+
203+
expect(upsert).not.toHaveBeenCalled();
204+
expect(del).not.toHaveBeenCalled();
205+
206+
// Control: same subscription, same row, the name the engine publishes.
207+
await deliver(CREATED);
208+
209+
expect(upsert).toHaveBeenCalledTimes(1);
210+
expect(upsert).toHaveBeenCalledWith('task', ROW);
211+
});
212+
213+
it.each([
214+
['payload.id', { id: 'task_1' }],
215+
['payload.record.id', { record: { id: 'task_1' } }],
216+
])('record.deleted (id from %s) deletes nothing, while data.record.deleted deletes the same id', async (_shape, payload) => {
217+
const harness = makeCtx();
218+
const { service, deliver } = await harness.boot(new KnowledgeServicePlugin());
219+
const upsert = vi.spyOn(service, 'handleRecordUpsert').mockResolvedValue(undefined);
220+
const del = vi.spyOn(service, 'handleRecordDelete').mockResolvedValue(undefined);
221+
222+
await deliver(legacy('record.deleted', payload));
223+
224+
expect(upsert).not.toHaveBeenCalled();
225+
expect(del).not.toHaveBeenCalled();
226+
227+
// Control: same subscription, same id, the name the engine publishes.
228+
await deliver(DELETED);
229+
230+
expect(del).toHaveBeenCalledTimes(1);
231+
expect(del).toHaveBeenCalledWith('task', 'task_1');
232+
});
233+
});

‎packages/services/service-knowledge/src/knowledge-service-plugin.ts‎

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,14 @@ export interface KnowledgeServicePluginOptions {
2121
*/
2222
sources?: KnowledgeSource[];
2323
/**
24-
* Subscribe to ObjectQL `record.*` events from `IRealtimeService`
25-
* for `object` sources. Defaults to `true`. Set to `false` to
26-
* disable inline event sync (e.g. when an external indexer drives
27-
* upserts).
24+
* Subscribe to ObjectQL `data.record.created` / `data.record.updated` /
25+
* `data.record.deleted` events from `IRealtimeService` for `object`
26+
* sources. Defaults to `true`. Set to `false` to disable inline event
27+
* sync (e.g. when an external indexer drives upserts).
28+
*
29+
* The aggregate `data.records.updated` / `data.records.deleted` events a
30+
* predicate write (`multi: true`) publishes carry a count, not records:
31+
* they cannot be synced and only log a warning that the index may be stale.
2832
* @default true
2933
*/
3034
enableEventSync?: boolean;
@@ -159,12 +163,18 @@ export class KnowledgeServicePlugin implements Plugin {
159163

160164
// [#4626] `data.record.*` payloads ARE the spec's `DataEvent`
161165
// (`@objectstack/spec/api`): the record body lives in `after`, the id
162-
// in the required top-level `recordId`. Reading the payload itself as
163-
// a record (what the shared branch below did) indexed the ENVELOPE —
164-
// `{ recordId, after }` — as if it were the row, so object sources
165-
// were syncing documents with none of the record's fields, and a
166-
// delete never resolved an id at all. Kept separate from the legacy
167-
// `record.*` shape rather than merged behind fallbacks.
166+
// in the required top-level `recordId`. The payload itself is the
167+
// ENVELOPE, never the row — reading it as a record indexes
168+
// `{ recordId, after }` as if it were the row (a document with none of
169+
// the record's fields), and a delete never resolves an id at all.
170+
//
171+
// These three are the only per-record names the ObjectQL engine
172+
// publishes. There is deliberately no `record.created|updated|deleted`
173+
// branch: no producer emits those names (the spec's `RealtimeEventType`
174+
// dropped them for the same reason), so a branch for them is dead code
175+
// plus a `payload.record ?? payload` fallback for a shape nobody sends.
176+
// An event type this handler does not name — those, and `metadata.*`
177+
// too — falls through without a sync.
168178
if (type === 'data.record.created' || type === 'data.record.updated') {
169179
const record = payload.after as Record<string, unknown> | undefined;
170180
if (record && typeof record === 'object') {
@@ -214,21 +224,6 @@ export class KnowledgeServicePlugin implements Plugin {
214224
);
215225
return;
216226
}
217-
218-
if (type === 'record.created' || type === 'record.updated') {
219-
const record =
220-
(payload.record as Record<string, unknown> | undefined) ?? payload;
221-
if (record && typeof record === 'object') {
222-
await service.handleRecordUpsert(object, record as Record<string, unknown>);
223-
}
224-
return;
225-
}
226-
if (type === 'record.deleted') {
227-
const recordObj = payload.record as Record<string, unknown> | undefined;
228-
const id =
229-
(payload.id as string | undefined) ?? (recordObj?.id as string | undefined);
230-
if (id) await service.handleRecordDelete(object, id);
231-
}
232227
});
233228
ctx.logger.info?.('KnowledgeServicePlugin: event sync subscription active.');
234229
});

‎packages/services/service-knowledge/src/knowledge-service.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,10 @@ export class KnowledgeService implements IKnowledgeService {
229229
// ── Sync entrypoints (called by the host plugin's event bridge) ───
230230

231231
/**
232-
* Apply an ObjectQL `record.created` / `record.updated` event to
233-
* every `object` source bound to the matching object. Failures are
234-
* logged but never thrown — sync must not block writes.
232+
* Apply an ObjectQL `data.record.created` / `data.record.updated` event
233+
* (the record body is the event payload's `after`) to every `object`
234+
* source bound to the matching object. Failures are logged but never
235+
* thrown — sync must not block writes.
235236
*/
236237
async handleRecordUpsert(object: string, record: Record<string, unknown>): Promise<void> {
237238
const targets = this.sourcesForObject(object);
@@ -249,7 +250,7 @@ export class KnowledgeService implements IKnowledgeService {
249250
}
250251
}
251252

252-
/** Apply an ObjectQL `record.deleted` event. */
253+
/** Apply an ObjectQL `data.record.deleted` event (the id is the payload's `recordId`). */
253254
async handleRecordDelete(object: string, recordId: string): Promise<void> {
254255
const targets = this.sourcesForObject(object);
255256
for (const source of targets) {

0 commit comments

Comments
 (0)