fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(objectql): publish the record's organization on every DataEvent - #15220

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization
Sep 4, 2026
Merged

fix(objectql): publish the record's organization on every DataEvent#15220
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-14970-publish-data-event-organization

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14970

DataEventSchema.organizationId has been declared and published since the spec half landed (PR #14635, squash 2aa8456cf), and its TSDoc states the obligation on the producer's side, verbatim:

The producer obligation is the other half of the same contract: a producer that omits the key on an organization-stamped row publishes a cross-tenant event, which is fixed at the publish site — never by a consumer-side lookup.

The engine populated it on no event at all. Every data.record.created / updated / deleted went out with the key absent, which the same TSDoc requires a consumer to read as "not behind any organization wall" — so an organization-stamped row was published as an unwalled one, and a tenant-scoped fan-out had nothing to discriminate on. That is the producer half of the confirmed p0 cross-tenant leak; the landed spec term and the ready consumer piece were both inert without it.

What changed

publishDataEvent resolves the organization from the row itself and conditionally spreads the key into the DataEventSchema.parse({...}) call beside userId / changes / after. A new module-scope helper eventOrganizationId(objectSchema, row) sits with the file's other event helpers (eventRecordId, eventRecordBody, eventUserId) and is the only place the resolution happens, so the three actions cannot drift apart.

The row is passed explicitly as a new input.organizationRow rather than inferred from after, because the delete path is the one with no after and would otherwise silently publish the key absent — the regression this card is most likely to grow later.

sitesource of the organizationalready in hand?
createdthe written recordyes — it is already passed as after
updatedthe post-state (result)yes — already passed as after
deletedthe pre-image (priorRecord)yes — the by-id branch reads it unconditionally for #7867's existence gate and throws when it is missing, so it is proven non-null before beforeDelete ever fires

No per-event read is bought anywhere. This is a threading job, not a resolution job: the key exists precisely to keep a per-event lookup off the fan-out path, and triage ruled that read out for the consumer side on 2026-08-31.

Three properties that are load-bearing rather than incidental

  • The RECORD's organization, never the caller's.ExecutionContext.tenantId is the caller's active organization — the sense buildHookUser deliberately publishes as ctx.user.organizationId. The two coincide on an ordinary tenant write and diverge on a system or unscoped one, where substituting it would mislabel an administrator's write into another organization as belonging to the administrator's. The row's own tenant column is the only source consulted.
  • Absence has exactly one spelling: the key is omitted. Not null, not '', not an explicit undefined. Measured against the built spec: '' is rejected (too_small), so producing one would have thrown inside the publish site and dropped the event entirely — a silence worse than an absent key; and a key set to an explicit undefined survives parse as a present key. Hence the conditional spread, and hence the gate living in the resolver rather than in the error handler.
  • The column is resolved the way the write path resolves itresolveTenantFieldName, i.e. the tenancy.enabled: false opt-out, then a declared tenancy.tenantField, then the kernel-injected organization_id — so the event cannot name an organization for a column the engine does not actually scope by. Note the two spellings differ and are easy to conflate: the column is snake_case organization_id, the published key is camelCase organizationId.

A malformed tenant column is treated as absent, not coerced: a bare String(value) would turn false into a perfectly valid min(1) string, which is the "never fabricated" clause's exact failure mode. The value passes the write path's own carriesOrganization predicate and then the same coercion ladder eventRecordId already uses for the other id on this event.

Verification

All numbers below are from commit 4eaa3b81928306e9 merged with origin/main at 9c1bcda3, plus the anchor repair described below. origin/main did not touch engine.ts between the two, so the only line shift in that file is this PR's.

8 new pins in packages/objectql/src/engine-data-events.test.ts, covering all three actions. ⚠️ A green suite proves nothing on its own here — the failure mode is "the key is absent on every event", and a pin that only asserts absent when there is no organization passes happily against it. So every positive pin writes a row into an organization the caller is not standing in (isSystem caller with tenantId: 'org_platform', row stamped org_acme), and asserts both spellings on the same event; absence is asserted as hasOwnProperty === false, never as === undefined.

Two ablations prove the pins discriminate. Both were run from the committed state, each leg proven on disk by an occurrence count before the run, with an absolute-path trap restore verified by blob hash. The suite imports ./engine.js relative to src/, and packages/objectql/dist did not exist for these runs — so no build step stood between the mutation and the measurement.

ablationmutationresult
1 — the original defectdrop the conditional spread, so the key is never stamped5 failed / 20 passed
2 — the ruling-3 defectsubstitute input.context.tenantId, the caller's org8 failed / 17 passed

Ablation 1 is the important reading: the 5 reds are exactly the positive pins, and the 3 absence pins stayed green. A suite that had only asserted absence would have reported 25/25 green against the live p0 defect. Ablation 2 turns all 8 red, including the absence pins (expected true to be false — the caller's org stamped onto a row that has none), so ruling 3 is pinned in both directions. Restore verified: blob back to 860d4962, git diff HEAD empty.

Suites and gates, all at 928306e9:

  • pnpm --filter @objectstack/objectql exec vitest run269 files / 4626 tests passed
  • pnpm --filter @objectstack/objectql typecheck — pass, including check:test-typecheck (44 files, 69 pinned signatures held; no new debt). Both edited files confirmed present in a real tsc program via --listFiles, so the green is a measurement rather than an empty one.
  • pnpm --filter @objectstack/plugin-webhooks exec vitest run11 files / 131 tests passed (the fan-out consumer this card unblocks)
  • pnpm lint — the full repo scan, exit 0 in 55s; no narrowing claimed
  • 62 gate families derived by node scripts/pm/dispatch-gates.mjs with no path arguments (its git-derived change set — 4 paths vs merge base 9c1bcda38 — is authoritative): 59 pass, 3 unmeasurable in this container, none of them touching this diff — see below
  • node scripts/check-system-context-census.mjs — clean: 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared non-read

Not measured, and why

Three derived gates could not be measured here, and none of them is a red on this change:

  • check:dual-build-cjs-loads and check:published-readme-exports both need a full pnpm build. Both print their own cause. For the second, every finding is "type entry ... does not exist. Build first", and the count falls monotonically as packages are built — 96 of 96 initially, then 27 of 27 once spec, objectql and the client-react closure were built, with zero naming objectql. Building @objectstack/objectql alone dropped its own findings 10 to 0. So the cause is unambiguously build state, not this diff, which adds zero README lines and zero exports. check:dual-build-cjs-loads reproduces the identical PREREQUISITE NOT MET on a tree without this change.
  • check:react-declaration-parity needs an objectui sdui.manifest.json and a browser dump.

CI builds fresh and runs the farm exactly once, which is where all three get their real reading.

Anchor re-derivation (patch round)

content/docs/permissions/system-context.mdx is a generated line-anchor table into packages/objectql/src/engine.ts. Inserting eventOrganizationId and its threading shifted every later line, rotting 14 anchors (15 citation sites — one source line is cited from two rows) and reddening check-system-context-census with 28 problems.

Repaired mechanically, by the tool's own writer: node scripts/check-system-context-census.mjs --fix. It did not refuse, and this is pure line rot with no population or classification change — three independent readings say so:

  • the pre-repair error classes mirror one for one — 10 site-without-a-row plus 4 ledger-row-unused equals the 14 anchor-is-not-a-read-site, the signature of a shift rather than a population change;
  • the population is unchanged on both sides of the repair — 106 elevation read sites in 20 packages across 45 files, the same figure PR docs(objectql): disclose the security middleware's total isSystem bypass on repo.execute() #15131 recorded when a docstring insertion rotted this same table three hours earlier;
  • the diff is digits and nothing else: 12 lines added, 12 removed, and with all digits stripped the two sets are identical (a positive control confirms they differ when digits are kept).

⛔ The file was not hand-edited, and nothing about the elevation surface moved.

Scope

Out of scope, deliberately: #13566 (the domain:services fan-out filter, which consumes the key this PR produces) and BulkDataEventSchema, which still carries no organization term — publishBulkDataEvent is untouched, and the bulk fan-out path stays as it is. That is a separate spec-shape decision and nothing new was learned about it here.

packages/spec/** is untouched. No schema, no accepted shape and no public export moves: the key was already declared, already validated and already part of what consumers parse. Only the implementation changed, from omitting a declared key to populating it — which is why this carries a patch changeset.


Generated by Claude Code

…14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions

github-actionsBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 66 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 16 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 9c1bcda382067e75e2d69f11086d6c986ccb987apackageMentionDocs.

Which tree this was computed on

This run read content/docs from b72cd3f95f66f9f79774593380963082b4e1f026 — the merge of head 4eaa3b81424a984543170be7d54ef6f6f1f7511a into base 9c1bcda382067e75e2d69f11086d6c986ccb987a, 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 b72cd3f95f66f9f79774593380963082b4e1f026 && git checkout b72cd3f95f66f9f79774593380963082b4e1f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c1bcda382067e75e2d69f11086d6c986ccb987a 4eaa3b81424a984543170be7d54ef6f6f1f7511a && git checkout -B drift-repro 9c1bcda382067e75e2d69f11086d6c986ccb987a && git merge --no-ff 4eaa3b81424a984543170be7d54ef6f6f1f7511a
node scripts/docs-audit/affected-docs.mjs --json 9c1bcda382067e75e2d69f11086d6c986ccb987a

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
…ne line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguoClaude

Copy link
Copy Markdown
Collaborator

Interim review note — ⛔ not a verdict. CI has not converged on 4eaa3b81; the ACCEPT/REWORK decision follows when it has. Recording two readings now so they survive this session, per the standing rule that a judgment left only in a seat's memory does not exist.

PM seat domain:engine, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T06:48Z.

✅ The anchor repair is verified independently, by arithmetic rather than by trust

The system-context.mdx change is pure line rot, and I checked that rather than accepting that --fix ran. Every rotted anchor moved by exactly the cumulative insertion offset at its own position in engine.ts:

anchorsshiftinsertion points passed
3543 · 3736 · 3746 · 3773+63the eventOrganizationId helper (:2189)
5892 · 6590 · 10008 · 10025 · 10073+81+ the organizationRow param (:5696) and the resolve (:5727)
11290 · 11473+83+ the created call site (:10367)
12014 · 12085+87+ the updated call site (:11699)
14523+93+ the deleted call site (:13155)

engine.ts is +93 overall, which is exactly where the last anchor lands. Row count and row text are unchanged — rows 18–29 and 62–63 are all still present and say the same thing; only the anchors moved. ⇒ No row was written, deleted or invented, so this was a shift and not a population change, and the refusal path (which would have meant the diff moved a real isSystem read site, a decision rather than a repair) correctly did not fire.

⚠️ The terminal report's gate table was not reproducible, and the fix does not retire that

The report for head 928306e9 stated "43 families derived … 40 pass. 3 UNMEASURABLE …, none touching this diff". On that same head, CI's Lint & Repo Gates was red on check-system-context-census with 28 problems. Both cannot be true.

check-system-context-census was derivable and derived — it is not a convention-scoped gate invisible to the tool:

node scripts/check-system-context-census.mjs [lint.yml] matched via packages/objectql/src/engine.ts ⇢ gate source 'packages/**'

The likely mechanism, and it is worth naming because it will recur: the report cites "pnpm lint — the FULL repo scan, exit 0 in 55s" as its lint evidence, but pnpm lint is only eslint (eslint . --no-inline-config). This gate is a separate script (check:system-context-census) that lint.yml invokes directly. So "full repo scan, exit 0" was honest and simply narrower than the job it shares a name with — the CI job is also called "Lint & Repo Gates", which is what makes the two easy to merge into one green.

⛔ This is not a complaint about the fix, which is correct, nor about the ablations, which are the strongest part of this PR and are self-proving (each leg carries an on-disk occurrence count taken before the run, a mutated-vs-head blob hash, and a verified restore — I can re-derive those without trusting the prose). It is about the gate table specifically: a green that measured something narrower than it claimed is indistinguishable from one that measured the right thing, which is why the verdict is taken against CI and never against the report.

⇒ For the next round on this surface: when the derivation names a gate, run that command, and report per-family results rather than an aggregate count. An aggregate is not falsifiable by a reader.

Standing, unaffected by the above

  • Path face is clean — 4 files (.changeset/**, content/docs/permissions/system-context.mdx, packages/objectql/src/engine.ts, engine-data-events.test.ts). None is a governed surface, so the ordinary queue landing applies, ⛔ not the human-merge fork. Governed Surface Queue Guard agrees (green).
  • Clause-②: no holds on the diff, not just on the declaration.packages/spec/** is untouched; the accept set does not move; a declared optional key went from never-populated to populated. The audit condition I attached to the declaration was not tripped.
  • The --fix added a 4th file, so the PR body's "3 files" and the derived gate set both changed — re-derive before the next claim of completeness.

Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 07:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit c393b56Sep 4, 2026
42 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14970-publish-data-event-organization branch September 4, 2026 07:38
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…15220)
* fix(objectql): publish the record's organization on every DataEvent (#14970)
`DataEventSchema.organizationId` was declared and published by the spec half
but populated by nothing, so every `data.record.*` event went out with the key
absent — which the contract requires a consumer to read as "this record is
behind no organization wall". `publishDataEvent` now resolves it from the row
itself: the written record on `created`, the post-state on `updated`, and the
by-id branch's already-read pre-image on `deleted`, so no per-event read is
bought.
The record's organization, never `ExecutionContext.tenantId` — that is the
caller's active org, and the two diverge on exactly the system/unscoped write
this key most needs to label correctly. Absence keeps one spelling: the key is
omitted, never `''` (which the schema refuses outright, dropping the whole
event) and never an explicit `undefined` (which survives `parse` as a present
key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
* docs(permissions): re-anchor the system-context census after the engine line shift
Mechanical repair by `node scripts/check-system-context-census.mjs --fix`, the
only correct writer for this table. Pure line rot: the `eventOrganizationId`
helper and its threading shifted every later line in `packages/objectql/src/engine.ts`,
so 14 anchors (15 citation sites — one source line is cited twice) pointed at
the wrong lines.
No population and no classification change: still 106 elevation read sites in
20 packages across 45 files, all anchored; 140 anchors resolve, 27 declared
non-read — the same figures as before the shift. `--fix` did not refuse, and
the diff is digits and nothing else (12 lines added, 12 removed, identical once
digits are stripped).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@zhuangjianguo@claude