Skip to content

fix(messaging): stamp organization_id on flow-produced notifications and markRead receipts - #11698

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-11303-stamp-organization-id-on-platform-writes
Aug 24, 2026
Merged

fix(messaging): stamp organization_id on flow-produced notifications and markRead receipts#11698
os-sam merged 1 commit into
mainfrom
claude/issue-11303-stamp-organization-id-on-platform-writes

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #11303

Stamps organization_id on the notification family at the two producers that
were not supplying it. Scope is narrower than the card — see What this PR
does not cover
below; the card should stay open.

Ruling

Maintainer, 2026-08-24, comment 5393621706, verbatim:

11303 sys_inbox_message/sys_notification/sys_email 应该写 organization_id。

A gap, not a design choice, and the producers are the fix site.

What the census found

The chain below the messaging ingress was already threaded end to end.
MessagingService.writeEvent stamps sys_notification, the inbox channel
stamps sys_inbox_message and its delivered receipt, and the outbox carries
the value onto the sys_notification_delivery row. Every one of them reads
EmitInput.organizationId.

The break was at the origin. The notify flow node — the dominant producer
of all four tables — never passed organizationId, and its local structural
mirror of emit() did not even declare the field, so the value could not have
been passed. One missing argument, four tables at 100% null. The other emit
caller in the tree, plugin-audit's collab.mention producer, already passes
it.

A second producer of the same table was found alongside it:
MessagingService.markRead inserts the read receipt — written when a user
reads a notification whose delivered receipt never landed — and that insert
named no organization at all. One table, two producers; a per-producer suite
that enumerated only the first would have gone green with this one still
emitting nulls.

The change

No fallback limb in either producer, deliberately. Not "the current
organization", not the install's first organization, not the recipient's first
membership. A wrong organization_id is worse than a null: a null is visibly
missing, while a wrong value is silently authoritative to every report, export
and cleanup script that filters by organization. Where no organization is in
scope, none is written.

Fail-loud, not fail-guess, and not fail-closed. The node warns audibly,
naming the topic and the consequence, rather than refusing. Refusing would break
the two deployments that legitimately have no organization to thread — a
single-posture install, and every stack before its first organization exists.
That trade-off has its own pin (PIN C below).

Forward-stamping only. No backfill, no migration, and no old row is
opportunistically stamped.

What this PR does NOT cover — why the card stays open

  1. sys_email — blocked on a fence, not attempted. Threading an
    organization to the sys_email writer requires widening SendEmailInput,
    which lives in packages/spec/src/contracts/email-service.ts. This card
    carries zero packages/spec ownership, so it stops here and reports
    rather than guessing at a public contract. plugin-email contains no
    organization handling of any kind today.
  2. sys_audit_log / sys_activity — the premise does not hold as written.
    All three writers (the CRUD writer, the auth-event writer, the read-audit
    writer) already stamporganization_id, through the shared platform-row
    organization resolver (Audit rows are stamped from the ACTOR's active organization in preference to the record's own — and the record-side fallback cannot see sys_api_key.active_organization_id #8707 / Promote resolveRecordOrganizationField to the shared platform-row resolver (approvals + automation runs), per the ruled cloud#1395 Option A #10101). Their residual nulls are writes where
    no organization was resolvable at all, which is exactly the case the ruling
    forbids filling by fabrication. No change was needed and none was made.

Because of (2) this PR touches nosys_audit_log declaration or writer, so
it does not overlap #11676.

Verification

All measurements below were taken on 8321b6a8, a clean tree at the final
commit.

Test-first. Every pin was written and run red on an otherwise-unmodified
tree, with its failure signature predicted in writing beforehand; no ablation
and no restore was involved, so no restore could silently fail. Observed red,
matching the predictions:

  • PIN A (producer identity, notify node) — expected undefined to be 'org_pin_alpha'
  • PIN B (end-to-end, zero org-less rows) — expected [ 'sys_notification:NULL', …(2) ] to deeply equal [ …(3) ]
  • PIN B2 (durable delivery row) — expected undefined to be 'org_pin_alpha'
  • PIN D (fail-loud) — no organization warning present
  • PIN E (markRead receipt) — expected undefined to be 'org_pin_beta'
  • PIN C and PIN E2, the over-denial controls, were green before and after: a
    stack with no organization in scope still delivers and still writes its rows.

PIN B asserts an identity list (object:organization_id per row in write
order), not a count, so an offsetting error cannot hold it green.

Suites (8321b6a8):

  • @objectstack/service-messagingTest Files 27 passed (27), Tests 276 passed (276)
  • @objectstack/service-automationTest Files 87 passed (87), Tests 1036 passed (1036)
  • @objectstack/service-messagingtypecheck — clean

Gates, derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
on the clean tree at the final commit (no path arguments), every exit code
captured before any pipe. All green; each gate's own verdict line:

  • check-engine-double-contract: OK — 398 pinned, 133 in the DEBT ledger, 2 exempt.
  • ✓ where-matcher conformance holds: 293 matcher(s) discovered … none new. / baseline key set verified against 8bcd054: no files added.
  • ✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new / baseline key set verified against 8bcd054: no files added.
  • check-type-check-coverage: OK — 65/78 workspace packages type-checked (plus the root), 13 in the DEBT ledger, 1 exempt.
  • check-nul-bytes: OK (scanned 6524 text file(s) … no raw ASCII control bytes).
  • check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).services/service-messaging in sync (4 bundle(s))
  • Also green: check:changeset-gate-self-tests, check:cross-package-test-inputs,
    check:objectui-changeset, check:published-files, check:slot-lookup,
    check:test-source-alias, check:type-source-resolution,
    check-adr-0087-registration, check-changeset-no-major,
    check-empty-changeset, check-plugin-teardown-shape,
    check-affected-docs, release-rehearsal-clone --self-test.

Neither ratchet moved: both report no files added against 8bcd054.

Declared narrowing.check:type-check-debt was not run locally — it demands
a full workspace build (turbo run build over every package) before it will
measure, and refuses outright otherwise. A refusal is NOT MEASURED rather than a
pass, so it is named here as not run rather than reported green; CI owns that
run. Repo-wide pnpm lint is likewise CI's run.

The two new test doubles declare no update() / delete() verb, so they add no
engine-double pin and leave the coverage ledger untouched.

Clause ② assessment: still NO. The change stamps a column on write. It
alters no accept/reject behaviour at any write door, widens no public surface,
and touches no packages/spec path — which is precisely why the sys_email
half stopped at the fence instead of proceeding.

Generated by Claude Code


Generated by Claude Code

…and markRead receipts
`sys_inbox_message`, `sys_notification`, `sys_notification_receipt` and
`sys_notification_delivery` were measured at 100% `organization_id = NULL` on a
live install, while `sys_approval_request` in the same database carried an
organization on every row. Ruled a gap, not a design choice.
The chain below the messaging ingress was already threaded end to end — every
writer reads `EmitInput.organizationId`. The break was at the origin: the
`notify` flow node never passed it, and its local structural mirror of `emit()`
did not declare the field, so it could not have. The node now threads the
organization from the run's acting context (`AutomationContext.tenantId`), the
same source `plugin-audit`'s `collab.mention` producer already uses.
A second producer of the same table is fixed alongside: the `read` receipt
`markRead` inserts named no organization at all, and now carries the
organization of the `sys_notification` row it is about.
No fallback limb in either producer, by design: a run with no organization in
scope still emits and still writes, and the node warns audibly instead of
guessing. A wrong organization_id is worse than a null — a null is visibly
missing, a wrong value is silently authoritative.
Forward-stamping only; no backfill and no migration.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/service-automation, @objectstack/service-messaging, touching 5 documentable anchor(s).

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

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

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 6 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 c6a62d01cd0d97a0bf0456aee59317330db0b3b6packageMentionDocs.

Which tree this was computed on

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

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

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@os-samClaude

Copy link
Copy Markdown
Collaborator

PM: docs-drift advisory dispositioned — no action

domain:services seat (session session_01APWX2AwT3a4xDcjPCe8bk4). Read with git grep <ref> on 55b9768761… — the merge commit the run itself named — not the shared working tree, which is a different and unknown state. Full review waits on the dev's report; this dispositions the advisory only.

The advisory lists 23 hand-written pages (truncated above 15 rows). Aimed at the two things this diff could actually falsify:

① Does any doc assert these tables carry no organization, or are not org-scoped? Positive control first: 7 non-release pages name sys_notification / sys_inbox_message on that ref, so the channel reaches them. No such claim exists — no page ties those tables to an absent organization, an unscoped read, or a null column.

② Does any doc describe the notify node's authorable inputs in a way this change makes wrong?No. The node is documented in several places — automation/flows.mdx, automation/workflows.mdx, automation/email-templates.mdx, concepts/architecture.mdx, getting-started/common-patterns.mdx, and its config table at references/automation/io-node-config.mdx:104 (topic | string | optional | Event topic (default: "notify")). Every one of them is an authoring-surface description, and this change adds no authorable input: the organization is threaded from the run's own acting context (AutomationContext.tenantId), never authored. So the documented config surface is unchanged and remains accurate.

⛔ The 4 release-owned pages were read-only and untouched.

⚠️ A methodological note, because the first answer to ② was wrong

My first pattern for ② returned empty, and I did not take that as an answer — it had no positive control. Running one showed content/docs/automation/ mentions notify heavily (flows.mdx 14 times, approvals.mdx 4, and six other pages). The empty result was a bad pattern, not an absence. The re-run with a working pattern is what produced the finding above.

Recording it because this is the third time today that "a zero-hit needs a positive control first" has changed a conclusion in this lane, and because a wrong zero here would have read as "the notify node isn't documented, so nothing can drift" — which is the opposite of true.

⚠️ For future readers of that bot generally: it is symbol-anchored and precision-first (#9192), covering hand-written docs only. It lists a page because the page names a symbol the diff touched, and it cannot see whether prose went false — which is why the check above was aimed by hand at two specific failure modes rather than derived from the row count.


Generated by Claude Code

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

Development

Successfully merging this pull request may close these issues.

1 participant

@os-sam