Skip to content

fix(service-automation,service-messaging,plugin-approvals,objectql)!: a notification takes the organization of what it is about, and is refused where no owner is derivable - #23340

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-23284-notify-takes-record-organization
Oct 13, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-23284-notify-takes-record-organization

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #23284
Clause-②: no (narrowing) — rows gain the organization they should already carry. The narrowing, which the seat chose where triage left it open: an emit about no record, from a context with no organization, gets the owner ADR-0131 D9 derives for a system write (the sole organization), and where D9 derives none it is refused loudly instead of stored with NULL.

What this changes

A notification now carries the organization of what it is about, at every producer, and a notification no organization owns is refused instead of stored with organization_id NULL.

  • notify node (service-automation). A run that carries an organization emits exactly as before. A run that carries none (a schedule tick, a record change written by a system context) takes the organization of the record the notification is about: the declared click-through record (sourceObject + sourceId), else the run's trigger record. The value is read from the STORED row by id under a system read, through the shared wall-column resolver (createRecordWallOrganizationResolver). It is never read from the record snapshot the run carries. The [notify] no organization in scope WARN is removed, together with the NULL row it predicted. When the engine refuses the emit, the step fails as notify failed: ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED: ....
  • Approvals (plugin-approvals). notify() takes a REQUIRED organizationId key, so no call site can leave it out. All 13 call sites pass the request's organization: requestOrg while the request opens, otherwise the stored row's organization_id. Before this change no approval notification carried one, including a user-driven approval.requested.
  • The engine rule reaches sys_notification (objectql). It is admitted to the design: isSystem 写入是否在租户审计控制范围内?——#13178 类级装置(A/B/C)的共同前置,从未被裁过 #13491 platform-object tenancy ledger as tenant-scoped. An emit that names no organization therefore gets ADR-0131 D9: the sole organization under single, and ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED (500) where none is derivable. This is the engine's own path, with no notify-specific copy of the rule.
  • The family follows its event row (service-messaging). writeEvent returns the organization the stored event row carries. emit() stamps that organization on the delivery, the inbox row and the receipt when the producer passed none. Otherwise the inline fan-out would write an owned event over org-less inbox and receipt rows.

Measured on a booted composition (before → after)

Harness: bootStack (@objectstack/verify, sqlite-wasm). It runs the production single composition with its Default Organization, plus record-change triggers and the approvals plugin, with reliable delivery ON (outbox + dispatcher), the HotCRM shape. The six emits:

  1. a schedule-tick notify about a record (sourceObject/sourceId);
  2. a schedule-tick notify about no record (the HotCRM case_sla_monitor shape);
  3. a record-change notify fired by a system-context insert (the task_urgent_alert shape);
  4. an approval node opened by a system insert;
  5. the same record-change flow from a REST create by a signed-in user;
  6. an approval node opened by a REST create by a signed-in user.

The schedule-tick context is the one ScheduleTrigger builds for an organization-less flow (event: 'schedule', no tenantId), handed to automation.execute. Before = cc14657ede; after = this branch's code, built.

single, one organization (HotCRM): NULL organization_id rows / total

before after
sys_notification 5 / 6 (all but the user-driven task alert; the user-driven approval.requested included) 0 / 6
sys_notification_delivery 0 / 6 0 / 6
sys_inbox_message 0 / 6 0 / 6
sys_notification_receipt 0 / 6 0 / 6
[notify] no organization in scope WARN lines (both topologies) 6 0

This matches the card's reading: NULL event rows, but inbox rows that carry the organization.

single, two organizations (D9 derives none):

before after
sys_notification NULL 5 / 6 0 / 5
deliveries / inbox rows / receipts written 1 / 1 / 1 (only the user-driven task alert; every other delivery was refused) 5 / 5 / 5, none NULL
schedule-tick notify about a record in org B step succeeded, NULL event row, nothing delivered event, delivery, inbox and receipt carry org B
system-written record change (task in org B) same all rows carry org B (trigger record)
system-opened approval (deal in org B) same approval.requested carries org B (the request's)
schedule-tick notify about NO record step succeeded, NULL event row stored, delivery refused silently step fails: Node 'n' failed: notify failed: ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED: Insert on 'sys_notification' was REFUSED: ... holds 2 organizations ...; nothing written

The PM's hypotheses, measured

  • H1: confirmed, with one correction. The node read context.tenantId only. The record it is about is available in two places: the node's own source (what the inventory anchors on) and the run's trigger record (AutomationContext.object + record). Both HotCRM notify nodes declare NO sourceObject/sourceId. case_sla_monitor notifies through actionUrl only, so from the platform's side its breach alert is about no record. The D9 arm, not the record anchor, is what gives it the Default Organization on HotCRM.
  • H2: falsified as worded. The explicit organization_id: null from writeEvent is NOT treated as carried (carriesOrganization(null) is false). D9 never saw the write because sys_notification was unclassified in the design: isSystem 写入是否在租户审计控制范围内?——#13178 类级装置(A/B/C)的共同前置,从未被裁过 #13491 ledger and so outside resolveSystemInsertOrganization. sys_notification_delivery IS admitted, which is why on HotCRM the delivery row, and the inbox row and receipt the dispatcher threads from it, carried the organization while the event row did not. Measured: before, one org, 0 NULL deliveries / 5 NULL events.
  • H3: confirmed. The approval.requested emit is in openNodeRequest. The helper never passed an organization at any of its 13 call sites.
  • H4: pinned. A run that carries an organization emits the identical input object (toStrictEqual, key for key) and issues no read, even when the click-through record lives in another organization (PIN I).
  • H5: the narrowing arm stands. Today, where D9 derives none, the record-less emit is ADMITTED (NULL event row stored) while its deliveries are refused, and the step reports success. After, the event write is refused and the step fails with the engine's code in its error text.
    • NodeExecutionResult.code is not set. That field is reserved for codes an executor classifies itself, and service-automation cannot import the engine's constant without a runtime dependency on objectql. The code reaches the step's error and the run's error instead.
    • Any other MessagingService.emit() producer gets the engine's rejection.
    • Walled (isolated/group) was measured at the engine level (classification pins). The open harness cannot boot a wall without the enterprise organizations plugin.

Pins

  • objectql tenancy-by-object-classification.test.ts: the admitted list now includes sys_notification. For it: single + one org derives; single + two orgs, isolated and group refuse with { code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 } and write nothing; a row carrying its organization is written as it came.
  • service-messaging notification-organization-follows-event.integration.test.ts, on a real ObjectQL over better-sqlite3:
    • an org-less emit on single + one org lands the sole organization on the event, inbox and receipt rows (inline) and on the delivery row (outbox);
    • on two orgs it rejects with the envelope and writes nothing;
    • an emit carrying its org is stored with exactly that one.
  • service-automation notify-organization-stamp.integration.test.ts:
    • PIN E: a schedule-tick notify about a record stores that record's organization on all four row kinds, and the read is one system read by id projecting only the wall column;
    • PIN F: the trigger record's STORED organization beats the snapshot's;
    • PIN G: source before trigger;
    • PIN H: fall-through order;
    • PIN C: record-less goes to emit() with no key and no read;
    • PIN D: a refusal fails the step with the code;
    • PIN I: user-driven is unchanged.
  • plugin-approvals approval-notification-body.integration.test.ts: the battery that drives all 12 notifying call sites now opens its deals in two organizations. Every emit's organizationId must equal the stored request row's.

Negative-pin ablations (committed first, trap-restored, blob-verified)

Both subjects are imported by relative path from src/ (./notify-node.js, ./approval-service.js), so no dist/ sits on the resolution path. Each mutation went through scripts/ablation-replace.mjs (anchor x1 → x0, blob changed), with a trap restore on EXIT/INT/TERM. Each restore was proven by the blob matching HEAD and an empty git diff HEAD.

1. Record anchor removed (organizationId = undefined && await organizationOfRecordAbout(...)), head b389aa57dd, notify-node.ts blob 2f4ea29ce0 → 37320cd67f → restored 2f4ea29ce0:

     × PIN E: a schedule-tick notify about a record stores THAT record's organization on all four row kinds
     × PIN F: a record change written by a system context takes the TRIGGER record's stored organization
     × PIN G: the declared click-through record is asked BEFORE the trigger record
     × PIN H: a record that names no organization falls through to the next anchor, then to none
AssertionError: expected [ 'sys_notification:NULL', …(2) ] to deeply equal [ …(3) ]
      Tests  4 failed | 7 passed (11)

2. Request-organization carry removed (the organizationId spread in notify() deleted), head b389aa57dd, approval-service.ts blob 5d330baff3 → edd34821d2 → restored 5d330baff3:

     × [#23284] every notification carries its request's organization, read back from the stored request
+   "approval.requested@thread: organizationId undefined, request \"org_battery_a\"",
+   "approval.requested@sla_notify: organizationId undefined, request \"org_battery_b\"",
    ... 16 offenders across all 12 notifying call sites
      Tests  1 failed | 3 passed (4)

Direction: red in both, as predicted.

Tests

  • Full package suites, run at d564b21d57 (the two test files changed after it were re-run at b389aa57dd, 4/4 and 4/4 green):
    • plugin-approvals: 74 files / 1052 tests green;
    • service-messaging: 50 / 568 green;
    • service-automation: 185 / 2455 green;
    • objectql: 413 / 8066 green.
  • Typecheck: at b389aa57dd the claim "green for all four packages" did NOT hold for service-automation: an orphaned recordingLogger helper in notify-organization-stamp.integration.test.ts failed TS6133, which the contract review 6156230712 caught. At 238d07b365 the CI step "Type check workspace packages" was run verbatim (pnpm exec turbo run typecheck over the 45 affected packages: 111 of 111 tasks, exit 0), and pnpm --filter @objectstack/service-automation typecheck exits 0.
  • Gates, at b389aa57dd: node scripts/pm/dispatch-gates.mjs --commands derived 70 commands from this diff (9 paths vs merge base cc14657ede). All 70 ran and each exited 0. --ran reconciled 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN, with every exit code recorded. This includes check:platform-object-tenancy-census, check:tenant-audit-census, check:system-context-census, check:adr-0087-registration (exemption no-migration-prescription accepted), check:query-options-erasure (test surface back at 236), check:slot-lookup, check:dual-build-cjs-loads (after building the 8 unrelated packages it found without dist/) and check-engine-split-ratio (after deepening the shallow clone as it prescribed).
  • Lint, narrowed: eslint --no-inline-config semantics (ESLint API, allowInlineConfig: false) over the 8 changed source files at b389aa57dd: 8 linted, 0 ignored by the config's own ignores, 0 errors, 0 warnings. Invariance: eslint.config.mjs sets no parserOptions.project and registers no typed rule, so no lint verdict is type-aware and this diff cannot move the verdict of any untouched file. The repo-wide pnpm lint is CI's.

Patch round 1 (at 238d07b365; contract review FAIL 6156230712, the dev's report 6156779695)

  • The red required check (TypeScript Type Check): the orphaned recordingLogger helper is deleted. PIN D asserts the step failure and the engine code in run.error, not a log line, so no assertion moved.
  • Semver: @objectstack/plugin-approvals is now minor, like the other three packages. The optional organizationId on ApprovalMessagingSurface.emit is reachable through the exported ApprovalServiceOptions.messaging and the public ApprovalService.attachMessaging. The changeset says so, where it used to say "not exported by name".
  • Gates at 238d07b365: 70 derived and 70 run, each exit 0. check:adr-0087-registration passes: [BREAKING+bang+clause-②-narrowing] not-required (no-migration-prescription). Suites: service-automation 2455 passed, plugin-approvals 1052 passed.

Acceptance notes

  • Contract-review point: the design: isSystem 写入是否在租户审计控制范围内?——#13178 类级装置(A/B/C)的共同前置,从未被裁过 #13491 ledger grows by one object. The pinned list says growing it is a maintainer decision. The admission cites the maintainer's 2026-08-24 ruling that names the table (「11303 sys_inbox_message/sys_notification/sys_email 应该写 organization_id。」), ADR-0131 D9 (column means in scope), and this PR's writers that fill the column. Only sys_notification is admitted. The inbox row and receipt follow the event row through emit(), so sys_inbox_message / sys_notification_receipt stay unclassified. That keeps a read receipt for a legacy NULL notification on a walled install writable until the v18 ceremony attributes the residue.
  • File surface beyond the claim: packages/objectql/src/tenancy/platform-object-tenancy.ts and its test (the engine path the seat's decision names). The engine.ts and config-parse files held by open PRs are untouched.
  • Stored residue is not touched. Existing NULL rows keep their attribution fate in the v18 ownership inventory.
  • HotCRM-side note, no platform change: case_sla_monitor and task_urgent_alert declare no sourceObject/sourceId. On HotCRM's one-organization install they now get the Default Organization (D9, and the trigger record). On an install holding several organizations, case_sla_monitor's alert would be refused until it names the case it is about.
  • Observation, not filed: on a single install holding two organizations, an organization-less schedule run's own sys_automation_run history row is refused. The engine logs this at ERROR (run-history persist failed ...), before and after this change. It is the run recorder's existing loud behaviour, in the engine file held by an open PR.

Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 13, 2026
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/objectql, @objectstack/plugin-approvals, @objectstack/service-automation, @objectstack/service-messaging, touching 18 documentable anchor(s).

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via approvals.getRequest (sdk, the route ledger binds it to GET /api/v1/approvals/requests/:id, selected by route anchor /approvals/requests/:id), getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/api/plugin-endpoints.mdx (via /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
  • content/docs/automation/approvals.mdx (via getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id), /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
  • content/docs/automation/flows.mdx (via /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
  • content/docs/automation/hook-bodies.mdx (via sys_notification (symbol, a field of const object PLATFORM_OBJECT_TENANCY))
  • content/docs/kernel/services-checklist.mdx (via /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
  • content/docs/permissions/system-context.mdx (via requestInfo (symbol, a method of class ApprovalService), sendBack (symbol, a method of class ApprovalService))

⛔ 4 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/releases/v17/17-0.mdx (via sys_notification (symbol, a field of const object PLATFORM_OBJECT_TENANCY), getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/releases/v17/17-2.mdx (via MessagingService (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via sys_notification (symbol, a field of const object PLATFORM_OBJECT_TENANCY))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 21 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 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 126f017fe019eb7feff9ea9eb1e02c981307a721 — the merge of head 238d07b36509b71682396b2fcc0451d6206899c9 into base 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8, 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 126f017fe019eb7feff9ea9eb1e02c981307a721 && git checkout 126f017fe019eb7feff9ea9eb1e02c981307a721
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 238d07b36509b71682396b2fcc0451d6206899c9 && git checkout -B drift-repro 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 && git merge --no-ff 238d07b36509b71682396b2fcc0451d6206899c9

node scripts/docs-audit/affected-docs.mjs --json 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8

⚠️ 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 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: b389aa57dd94d069645d728f9440f0b47819fe6a
Local-runs: none

Inputs, and nothing else: card #23284 (body and every comment: the triage ruling 6148637195, the claim 6154283897, the os-dev-report 6155779257, the claim amendment 6155816752); PR #23340 (body, file list, the net diff of the head against its merge base cc14657ede with main at 614fdec832); the check-runs on the head sha, read through the API after every run completed. Nothing built, run or re-run locally; the one log read is the red job's own, through the API.

① Derived judgments

  1. @objectstack/objectql — PLATFORM_OBJECT_TENANCY (exported from the package index) gains sys_notification: tenant-scoped. Accept-set change on a system-context insert('sys_notification', …) that names no organization on the context or the row: single with exactly one organization DERIVES it; single with two or more, isolated, group, and single with zero organizations where sys_organization is registered REFUSE with ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED (500) and write nothing; the engine's isSystem tenant-audit auto-mute no longer covers the object. Right. The ledger's own admission bar is a citable writer fact: the entry cites the maintainer's ruling that names the table verbatim — 「11303 sys_inbox_message/sys_notification/sys_email 应该写 organization_id。」, already quoted on main in notify-node.ts — plus the writers this diff makes stamp the column, the same shape as the sys_file and sys_approval_request entries beside it; ADR-0131 D9 is the rule it reaches. The pinned list grows by exactly one name and every arm is pinned in tenancy-by-object-classification.test.ts. ScheduleTrigger's BIND line on main already tells the operator that under single holding several organizations an undeclared flow's notification "will be REFUSED at the write"; this diff makes the event row do what that line says. Note: the zero-organization single arm is inside the narrowing, and the changeset's "Refused now" names only the walled and the several-organizations cases; its general sentence ("where no organization is derivable") covers it.

  2. @objectstack/service-messaging — MessagingService.emit() (public): (a) an emit that names no organization and whose event write the engine refuses now REJECTS with the engine's envelope (code plus status) and writes nothing, where main stored the event row with NULL and went on; (b) when the producer passed none and the stored event row carries an organization, that organization is stamped on the outbox delivery row, the inline inbox row and the receipt (owned), and is what Notification.organizationId hands every channel. Right — the narrowing is the card's "no notification row is organization-less", pinned on a real ObjectQL over better-sqlite3 for the inline and the outbox path, the two-organization refusal included. writeEvent (private) now returns { id, organizationId? } — internal. Observation, unchanged from main and outside the ask: audience resolution (resolver.resolve), the preference filter and the channel-availability consult still read the producer's absent organizationId, not the derived one (the consult runs before the write and cannot; the other two could). Not a finding against this head.

  3. @objectstack/service-automation — the notify executor: a run whose context carries no tenantId now issues ONE findOne by id under { context: { isSystem: true } } projecting ['id', wallColumn], first for the declared click-through record (sourceObject + sourceId), else the run's trigger record (context.object + context.record.id), through createRecordWallOrganizationResolver (@objectstack/metadata-core, already a dependency; the wall column, ADR-0066 opt-out honoured); an unknown object, a missing row and an empty column each fall through, a thrown read fails the step, and a run carrying tenantId emits the byte-identical input and reads nothing (PIN I). The [notify] no organization in scope WARN is gone with the NULL row it predicted. The step's failure text becomes notify failed: CODE: message for any thrown error carrying a string code, not only the refusal; NodeExecutionResult.code stays unset, reason in-code. Right — the triage direction is "the organization of the record it is about, the same anchor the inventory uses, never a guessed or default organization", and no limb here guesses. Pinned (PIN C–I) with the real engine and the real messaging service over a capturing data engine.

  4. @objectstack/plugin-approvals — ApprovalService.notify (private) takes a REQUIRED organizationId: string | null | undefined; all 13 this.notify({ sites at head pass requestOrg (while the request opens) or the stored row's raw.organization_id; null and empty normalise to an absent key, so the engine rule decides. Every approval notification now carries its request's organization, the user-driven approval.requested included. Right — the triage direction verbatim; the body battery opens its deals in two organizations and pins every emit's organizationId to the stored request row's. 12 of the 13 sites are driven (the second approval.reminder branch, for literal audience entries, is not); it passes the same raw.organization_id as its sibling, read off the diff. ApprovalMessagingSurface.emit's input gains an optional organizationId?: string. The changeset calls this "not exported by name": literally true (index.ts does not list the interface), but it is reachable from the entry type graph through the exported ApprovalServiceOptions.messaging and the public ApprovalService.attachMessaging(…), so it is published surface, not shipped bytes. Additive and optional, no consumer rewrites anything: right as a change; the changeset sentence under-states reachability — see ②.

② Semver level

  • Clause-②: no (narrowing) on the PR body, the claim's line copied verbatim. Right. The path limb is clear (no packages/spec/src/** file) and the declared limb is honest: no key is added to a published payload — sys_notification.organization_id gains VALUES in a column it already had, and the only new key is the optional one on the in-process messaging surface above. The (narrowing) arm is right and was measured before it was kept (H5): the accept set of sys_notification system writes, of MessagingService.emit() and of the notify step all shrink where no owner is derivable — BREAKING.
  • .changeset/23284-notification-takes-record-organization.md: objectql, service-messaging, service-automation at minor — right; a breaking change ships minor under the launch-window convention and check-changeset-no-major is green on this head. plugin-approvals at patch — wrong by the letter of the level rule: a purely additive widening of a published package's public surface takes at least minor (the rule check-changeset-no-major and the Check Changeset step state), and the optional organizationId on ApprovalMessagingSurface is reachable published surface (①-4). The published version is unaffected — the fixed group ships every package at the group's highest bump, minor — so this is a declaration error, not a release error. Patch round: grade plugin-approvals minor and rewrite the "not exported by name" sentence to say the key is reachable through ApprovalServiceOptions.messaging.
  • Body: the BREAKING banner, the owner rules, the "Refused now" paragraph and the remedy ("give the notification an owner": sourceObject + sourceId, organization on the start node, or organizationId to emit()) — the text an upgrading agent greps. ADR-0087 disposition not-required (no-migration-prescription): right — nothing authorable is removed, renamed or re-shaped and no stored row is rewritten; the remedy is authoring guidance for a runtime refusal, not a FROM → TO rewrite of a retired spelling; check:adr-0087-registration accepted it on this head (Lint & Repo Gates green).
  • Checks on the head, read through the API once every run had completed: 40 runs — 32 success, 6 skipped, 2 failure; all six Test Core shards, Build Core, Lint & Repo Gates, Check Changeset and Governed Surface Queue Guard green. Two are red, and the same-signature-on-base exception does not apply — the merge base cc14657ede has Type Check · workspace and TypeScript Type Check green (jobs 115516838615 and 115522226805), and the error names a symbol this diff orphaned:
    • Type Check · workspace (job 115553650674) — FAIL. Failing step: "Type check workspace packages" (type-check-workspace-packages); pnpm run typecheck in packages/services/service-automation exited 2. First error line: src/builtin/notify-organization-stamp.integration.test.ts(64,10): error TS6133: 'recordingLogger' is declared but its value is never read.
    • TypeScript Type Check (job 115557073665) — FAIL: the required context, red because the lane above is.
      Cause, read off the diff: the merge base calls recordingLogger() once, in the old PIN D; the rewrite of PIN D dropped that call and kept the helper, and the root tsconfig.json sets noUnusedLocals: true, so tsc --noEmit reds on it. The helper is already orphaned at d564b21d57, the sha the dev's report cites for "typecheck green for all four packages" — that reading does not hold for service-automation.

③ Boundary flags

  • open_questions[0] — is the ledger admission a maintainer decision? The claim amendment (6155816752) answered A; this record confirms A: the ledger header's admission bar is a citable writer fact, the entry cites the maintainer's ruling naming the table verbatim and the writers this diff lands, and only the one object is admitted — sys_inbox_message and sys_notification_receipt stay unclassified and follow the event row through emit(), which keeps a receipt for a legacy NULL notification writable until the v18 ceremony attributes the residue. The right scope; not escalated.
  • Deviation 1 (file surface beyond the claim: the objectql ledger and its classification test) — accepted by the claim amendment; the file list touches neither engine.ts (fix(spec,service-automation,metadata-protocol): the v18 stored preview lists every row protocol 18 refuses and no conversion rewrites #23315) nor builtin/config-parse* (fix(spec)!: a builtin config value carrying a {token} is judged for its type at the build doors (#23260) #23319). Answered.
  • Deviation 2 (the booted measurement harness is not committed) — the committed pins cover each seam on its own: the engine rule on a real ObjectQL, family-follows-event on a real ObjectQL, the node's anchors with the real engine and messaging service, the approvals battery. Accepted; no committed pin boots ScheduleTrigger itself, which deviation 4 covers.
  • Deviation 3 (bump levels) — judged in ②: three right, plugin-approvals to minor.
  • Deviation 4 (the schedule tick is hand-built, not a fired job) — verified against ScheduleTrigger on main: its literal is event: 'schedule', tenantId OMITTED for an undeclared flow, params: { jobId, flowName, schedule }, callerParamKeys: []; the test's SCHEDULE_TICK matches it key for key. Answered.
  • Deviations 5–8 (model-free commit trailers; the deepened clone and the eight dists built for two gates; an overlapped gate run superseded by the complete one; worktree cleanup) — local acts, nothing in the diff. Answered.
  • Out-of-scope (a): an organization-less schedule run on single holding two organizations has its own sys_automation_run row refused and logged at ERROR, before and after — the ruled loud behaviour (D9; ScheduleTrigger's BIND line predicts it), in engine.ts held by fix(spec,service-automation,metadata-protocol): the v18 stored preview lists every row protocol 18 refuses and no conversion rewrites #23315. Accepted as noted; no card.
  • Out-of-scope (b): HotCRM's case_sla_monitor and task_urgent_alert declare no sourceObject / sourceId; on an install holding several organizations the record-less alert is refused until the node names the case. Escalated to the repo:hotcrm seat with the landing note by the claim amendment. Answered.
  • Dev's tests field, "typecheck green for all four packages" — does not hold on this head (② above); the report's other readings (full suites at d564b21d57, the two ablations with blob-verified restores, 70 of 70 gate commands) are consistent with the diff and with the green runs.

The diff's semantics pass on every item above. The verdict is FAIL on the required check: TypeScript Type Check is red on this head for a cause inside the diff. Patch round: delete the unused recordingLogger helper in notify-organization-stamp.integration.test.ts (or give it a caller), grade plugin-approvals minor and correct the changeset's reachability sentence. Nothing else in the diff needs to move; a new record on the new head follows.

Implemented-by: claude/issue-23284-notify-takes-record-organization
Reviewed-by: session_01FzQy26KznSa5j2gnf3AzsE

VERDICT: FAIL


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 238d07b36509b71682396b2fcc0451d6206899c9
Local-runs: none

Re-review after the FAIL record 6156230712 on b389aa57dd. Inputs, and nothing else: card #23284 (body and every comment: the triage ruling 6148637195, the claim 6154283897, the round-1 os-dev-report 6155779257, the claim amendment 6155816752, the patch-round report 6156779695); PR #23340 (body, file list, every comment, and the net diff of the head against main — merge base cc14657ede, 9 files, +732/−84; origin/main read at 3490bf5fa5); the check-runs on the head sha, read through the API after every run had completed. The whole diff is judged at this head, not the delta alone; nothing was built, run or re-run locally.

① Derived judgments

  1. @objectstack/objectql — PLATFORM_OBJECT_TENANCY (exported) gains sys_notification: tenant-scoped. Accept-set change on a system-context insert('sys_notification', …) carrying no organization on the context or the row: single holding exactly one organization DERIVES it; single holding two or more, single holding none (with sys_organization registered), isolated and group REFUSE with ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED (500) and write nothing — the three arms the engine's own refusal text already enumerates; the engine's isSystem tenant-audit auto-mute no longer covers the object. Right. The ledger header's admission bar is a citable writer fact; the entry cites the maintainer's ruling naming the table verbatim — 「11303 sys_inbox_message/sys_notification/sys_email 应该写 organization_id。」 — and the writers this diff makes fill the column, the same shape as the sys_file, sys_upload_session and sys_notification_delivery entries beside it. The committed census (scripts/platform-object-tenancy-census.json) already lists sys_notification with reach in and tenantField organization_id, so the column the ledger speaks of exists and the census gate is unmoved. Exactly one name is added to the pinned list, and every arm is pinned in tenancy-by-object-classification.test.ts, the carried-organization control included. ScheduleTrigger's BIND text on main already tells the operator that an undeclared flow's notification "will be REFUSED at the write" on such an install; this diff makes the event row do what that text says.

  2. @objectstack/service-messaging — MessagingService.emit() (public): (a) an emit that names no organization and whose event write the engine refuses now REJECTS with the engine's envelope (code plus status) and writes nothing, where main stored the event row with NULL and went on — the dedup catch consults findEventByDedupKey and rethrows when there is no winner, so a refusal is not laundered into a dedup hit; (b) when the producer passed none and the stored event row carries an organization, owned carries it onto the outbox enqueue (delivery row) and onto Notification.organizationId (inline inbox row and receipt), so the family cannot split into an owned event over org-less children. Right — the card's "no notification row is organization-less", pinned on a real ObjectQL over better-sqlite3 for the inline and the outbox path, the two-organization refusal (nothing written) and the carried-organization control included. writeEvent (private) now returns { id, organizationId? }, the organization read off the row the engine returned — internal. Observation, unchanged from main and outside the ask: the pre-write channel-availability consult, resolver.resolve and preferences.filter still read the producer's absent organizationId, not the derived one; all three are reads, none writes a row. Not a finding against this head.

  3. @objectstack/service-automation — the notify executor: a run whose context carries no tenantId issues ONE findOne by id under { context: { isSystem: true } } projecting ['id', wallColumn], first for the declared click-through record (sourceObject + sourceId, the same source the event row stores), else the run's trigger record (context.object + context.record.id), through createRecordWallOrganizationResolver from @objectstack/metadata-core (already a dependency; ADR-0066 opt-out honoured, declared tenancy.tenantField then organization_id); the data engine is resolved exactly as crud-nodes.ts resolves its own ('data', else 'objectql'); an unknown object, a missing row and an empty column each fall through, a thrown read fails the step, a run carrying tenantId emits the byte-identical input and reads nothing. The [notify] no organization in scope WARN is gone with the NULL row it predicted. The step's failure text becomes notify failed: CODE: message for any thrown error carrying a string code (not only the refusal — a text change, no pinned text broke); NodeExecutionResult.code stays unset, the reason in-code. Right — the triage direction is "the organization of the record it is about, the same anchor the inventory uses, never a guessed or default organization", and no limb here guesses: when nothing answers, the emit carries no key and the engine's rule decides. Pinned PIN C–I with the real engine and the real messaging service over a capturing data engine; the SCHEDULE_TICK literal matches the handler literal in trigger-schedule/src/schedule-trigger.ts key for key (event: 'schedule', tenantId omitted for an undeclared flow, params: { jobId, flowName, schedule }, callerParamKeys: []).

  4. @objectstack/plugin-approvals — ApprovalService.notify (private) takes a REQUIRED organizationId: string | null | undefined; all 13 this.notify({ sites at head pass it — requestOrg at the three sites inside openNodeRequest (the very value the request row is stored with, organization_id: requestOrg), raw.organization_id at the other ten (returned ×2, reassigned, reminder ×2, request_info, comment, escalated, sla_breached ×2); null and empty normalise to an absent key, so the engine rule decides. A missing site would not compile, and TypeScript Type Check is green. Right — the triage direction verbatim; the body battery opens its deals in two organizations and pins every emit's organizationId to the stored request row's. ApprovalMessagingSurface.emit's input gains an optional organizationId?: string: the interface is not listed in index.ts, but it is reachable through the exported ApprovalServiceOptions.messaging and the public ApprovalService.attachMessaging(…) — published surface, additive and optional, no implementation has to change. The notify() helper's own catch keeps a refused approval emit at warn and returns 0: a functional degradation (the recipient visibly gets nothing; the request itself is tenant-scoped and was derived or refused at its own write), the right level.

② Semver level

  • Clause-②: no (narrowing) on the PR body and in the changeset, the same arm as the claim. Right. No key is added to a published payload — sys_notification.organization_id gains VALUES in a column it already had, and the only new key is the optional one on the in-process approvals messaging surface above. The (narrowing) arm is right and was measured before it was kept (H5): the accept set of sys_notification system writes, of MessagingService.emit() and of the notify step all shrink where no owner is derivable — BREAKING, ! on the title, **BREAKING** in the changeset body.
  • .changeset/23284-notification-takes-record-organization.md: all four packages at minor. Right — a breaking change ships minor under the launch-window convention, and the plugin-approvals grade, patch on the previous head, is now minor: a purely additive widening of a published package's public surface takes at least minor (the rule check-changeset-no-major.mjs states), and the optional organizationId on ApprovalMessagingSurface is reachable published surface (①-4). The "not exported by name" sentence is rewritten to say exactly that reachability. The fixed group ships every package at the group's highest bump, so the declaration and the release now agree.
  • Body: the owner rules, the "Refused now" paragraph and the remedy ("give the notification an owner": sourceObject + sourceId, organization on the start node, or organizationId to emit()) — the text an upgrading agent greps. ADR-0087 disposition not-required (no-migration-prescription): right — nothing authorable is removed, renamed or re-shaped and no stored row is rewritten; the remedy is authoring guidance for a runtime refusal, not a FROM-to-TO rewrite of a retired spelling; check:adr-0087-registration accepted it on this head (Lint & Repo Gates green).
  • Delta since the FAIL head, read as its own diff: two files, +2/−15 — the plugin-approvals grade and the reachability sentence in the changeset, and the deleted recordingLogger helper in notify-organization-stamp.integration.test.ts. Nothing else moved, so every judgment above on the unchanged files carries over and was re-read anyway.
  • Checks on the head, read through the API once every run had completed: 47 runs — 39 success, 8 skipped (path-filtered and opt-in lanes: Build Docs, Console Pin Gate, Temporal Conformance, the dogfood shards, Packed-tarball smoke, and the second workflow run's Auto Label / Check PR Size), 0 failure, 0 in progress. All five required contexts are success: Lint & Repo Gates, TypeScript Type Check, Test Core (all six shards), Build Core, Governed Surface Queue Guard. Check Changeset is green on both runs. The two runs red on b389aa57dd — Type Check · workspace and TypeScript Type Check — are success here; the cause named by the previous record (the orphaned helper under noUnusedLocals) is deleted in the delta above.

③ Boundary flags

  • Round-1 open_questions[0] — is the ledger admission a maintainer decision? The claim amendment (6155816752) answered A and the previous record confirmed it; confirmed again at this head: the ledger header's admission bar is a citable writer fact, the pinned-list comment's "a maintainer decision" is met by the maintainer's own ruling naming the table, the entry cites that ruling and the writers this diff lands, and sys_notification_delivery and sys_upload_session sit on the same ledger admitted on writer facts. Only the one object is admitted — sys_inbox_message and sys_notification_receipt stay unclassified and follow the event row through emit(), which keeps a receipt for a legacy NULL notification writable until the v18 ceremony attributes the residue. Not escalated.
  • Round-1 deviation 1 (file surface beyond the claim: the objectql ledger and its classification test) — accepted by the claim amendment; the file list touches neither engine.ts (fix(spec,service-automation,metadata-protocol): the v18 stored preview lists every row protocol 18 refuses and no conversion rewrites #23315) nor builtin/config-parse* (fix(spec)!: a builtin config value carrying a {token} is judged for its type at the build doors (#23260) #23319). Answered.
  • Round-1 deviation 2 (the booted measurement harness is not committed) — the committed pins cover each seam on its own: the engine rule on a real ObjectQL, family-follows-event on a real ObjectQL, the node's anchors with the real engine and messaging service, the approvals battery. Accepted.
  • Round-1 deviation 3 (bump levels) — resolved in ②: four at minor.
  • Round-1 deviation 4 (the schedule tick is hand-built, not a fired job) — the literal matches schedule-trigger.ts's handler key for key (①-3). Answered.
  • Round-1 deviations 5–8 (model-free commit trailers — all five commits on the branch carry the Claude-Session + Co-authored-by: Claude pair; the deepened clone and the eight dists built for two gates; an overlapped gate run superseded by the complete one; worktree cleanup) — local acts, nothing in the diff. Answered.
  • Round-2 deviation 1 (the round-1 "typecheck green for all four packages" claim did not hold for service-automation) — the correction is on the card; the check-run on this head is the verdict, and it is green. Answered.
  • Round-2 deviation 2 (a local turbo build at --concurrency=4) — a local act, nothing in the diff. Answered.
  • Round-2 deviation 3 (no merge of origin/main this round) — mergeable_state is clean, the PR runs tested the merge ref against base 430e5f5ad1, and origin/main has since moved to 3490bf5fa5; the queue rebuilds the PR as merged onto current main and is the control for that. Answered.
  • Round-2 deviation 4 (worktree recreated and removed) — local. Answered.
  • Out-of-scope (a): an organization-less schedule run on single holding two organizations has its own sys_automation_run row refused and logged at ERROR, before and after — the ruled loud behaviour (D9; the BIND text predicts it), in engine.ts held by fix(spec,service-automation,metadata-protocol): the v18 stored preview lists every row protocol 18 refuses and no conversion rewrites #23315. Not a defect; accepted as noted.
  • Out-of-scope (b): HotCRM's case_sla_monitor and task_urgent_alert declare no sourceObject / sourceId; on an install holding several organizations the record-less alert is refused until the node names the case. Escalated to the repo:hotcrm seat by the claim amendment. Answered.
  • Docs: no hand-written page enumerates the tenancy ledger, so none goes stale; content/docs/deployment/tenancy-modes.mdx ("every tenant-scoped insert beneath it resolves that one organization" under single; a notification "is refused" behind a wall) now holds for the event row as well. The docs-drift list is advisory and names no page this diff falsifies. No doc change owed.
  • The previous record's three patch-round items — delete or call recordingLogger, grade plugin-approvals minor, correct the reachability sentence — each resolved, as the delta in ② shows.

The diff's semantics pass on every item above, the declaration matches what the diff publishes, and every check on this head is green. The PR touches no governed surface, is 816 changed lines, is not a fork, and is still draft; landing is the owning seat's.

Implemented-by: claude/issue-23284-notify-takes-record-organization
Reviewed-by: session_01FzQy26KznSa5j2gnf3AzsE

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 13, 2026 22:58
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 13, 2026 22:58
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 13, 2026
Merged via the queue into main with commit 24e064f Oct 13, 2026
48 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-23284-notify-takes-record-organization branch October 13, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants