Skip to content

feat(service-messaging): sys_inbox_message carries actor_id, travelling end to end from emit() - #17886

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-16974-inbox-actor-id
Sep 12, 2026
Merged

feat(service-messaging): sys_inbox_message carries actor_id, travelling end to end from emit()#17886
os-project-manager merged 3 commits into
mainfrom
claude/issue-16974-inbox-actor-id

Conversation

@os-project-manager

@os-project-manager os-project-manager commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16974

Implements the director seat's ruling (#issuecomment-5643392717, decision batch #119 item 5, maintainer verbatim 「同意」 to 1A·2A·3A·4D·5A): option A — widen the channel seam and snapshot the actor onto the delivery row.

The path

An inbox row could not answer "did I cause this?". The actor stopped one layer upstream on sys_notification.actor_id, and the shipped default permission sets grant a member no read on sys_notification — so the value sat behind an FK hop into an object the reader cannot open. The actor now travels, one leg per seam, with no new read anywhere:

leg file change
1 channel.ts Notification.actorId?: string — the per-recipient unit every channel implementation consumes
2a (P0) messaging-service.ts emit()'s inline literal projects EmitInput.actorId onto it
2b (P1) messaging-service.ts enqueueDeliveries snapshots actorId into deliveryPayload, beside the rendered content
2b outbox.ts DeliveryPayload.actorId?: string declared, rather than left to that type's index signature
3 dispatcher.ts processRow reads p.actorId back off the snapshot
4 inbox-channel.ts actor_id: n.actorId ?? null on the row
5 objects/inbox-message.object.ts actor_id, Field.lookup('sys_user'), same semantics as sys_notification.actor_id

Item 2 — digest rows keep actor_id null by construction. processDigestGroup sets no actor: a collapsed group has no single actor, so "you caused this" must not be asserted over a message that also carries other people's events. Stated in the object's own description text and pinned with a same-tick control (below).

Item 3 — plugin-security/src/objects/default-permission-sets.ts is NOT touched, and that is the finding, not an omission. Re-measured on this branch's base: the file declares zero field-level permissions (grep -c 'fieldPermissions' = 0, grep -cE '^\s*fields:' = 0), and field permissions are a subtract-only mask — FieldMasker.maskResults returns results untouched when the permission map is empty, and hiddenFields is derived only from entries whose readable is false. The object-level sys_inbox_message: { allowRead: true, … } entry, narrowed to the recipient by the sys_inbox_message_self RLS policy, therefore already carries the new column. Item 3 is a no-op; editing the file would have been a speculative touch.

Item 4 — Clause-②: yes. Two widenings, both intended and both declared: the Notification seam type gains a member, and the stored sys_notification_delivery.payload JSON column gains a key. The changeset is a minor on @objectstack/service-messaging. ⛔ Item 5 (objectui#7011's consumer follow-up) is another repo's card and is untouched here.

Tests

New pins, all in files that already existed (no new engine double, no ledger row — see report):

  • inbox-channel.test.ts — the channel's own leg (actor on the row; null, asserted with in, when absent), plus one whole-path run: real MessagingService + MemoryNotificationOutbox + real NotificationDispatcher + real inbox channel, emit({ actorId: 'user_9' }) → delivery-row snapshot → tick()sys_inbox_message.actor_id === 'user_9', and an assertion that no findOne against sys_notification happened. Four green legs do not prove a connected path.
  • digest.test.ts — the read-back and the digest absence in one test with its own control: in the same tick the immediate row's actor survives the outbox round trip (user_9) while the collapsed group's actorId is undefined. An absence is evidence only when the same run shows the presence. Plus: a non-string actorId on a stored payload does not reach the typed seam.
  • messaging-service.test.ts — the P0 projection (same actor reaches every registered channel) and the P1 snapshot, each with its actor-absent twin.
  • inbox-channel.test.ts's existing whole-row toEqual key-set pin updated with actor_id: null — it is designed to go red on a key-set change and it did.

Ablation — every negative pin gets one

Both legs ran from the committed tree, each under a trap '<restore>' EXIT INT TERM with absolute paths; restoration is proven by an empty git diff HEAD and a git hash-object match against the HEAD blob, not by an exit code. Before each run: the anchor text was counted on disk before and after the mutation, and git diff --numstat confirmed the edit landed.

These tests exercise SOURCE, not dist. Same-package relative specifiers (./dispatcher.js) resolve to src/*.ts under vitest, and this package's vitest.config.ts aliases only @objectstack/core. The proof is the result itself: a source-only mutation with no rebuild turned the tests red, which a dist-resolved subject could not do.

Leg 1 — delete the dispatcher's read-back (actorId: typeof p.actorId === 'string' ? p.actorId : undefined, one line removed; anchor count 1 → 0, numstat 0 1):

× restores the actor on an immediate row and leaves a digest group without one
× carries the actor through emit → outbox snapshot → dispatcher → row (P1)
AssertionError: expected undefined to be 'user_9' // Object.is equality
AssertionError: expected null to be 'user_9' // Object.is equality
 Test Files  2 failed (2)
      Tests  2 failed | 31 passed (33)

⭐ Worth recording: the digest-absence assertion stayed green under this mutation. An absence assertion cannot distinguish "deliberately not set" from "the plumbing is broken", which is exactly why it ships with a same-tick presence control rather than alone.

Leg 2 — let the digest group claim the first row's actor (one line inserted into processDigestGroup; marker count 0 → 1, numstat 1 0):

× restores the actor on an immediate row and leaves a digest group without one
AssertionError: expected 'user_0' to be undefined
 Test Files  1 failed (1)
      Tests  1 failed | 8 passed (9)

Restore, both legs: git diff HEAD empty, git hash-object 083c99761dda01cba6b2ec059f3a3902e3ef95a3 == the HEAD blob.

Local verification

run verdict
pnpm --filter @objectstack/service-messaging test Test Files 41 passed (41) · Tests 441 passed (441) · lock VERDICT command-exit 0
pnpm --filter @objectstack/service-messaging typecheck lock VERDICT command-exit 0
pnpm --workspace-concurrency=2 --filter '...@objectstack/service-messaging' build Scope: 28 of 81 workspace projects, lock VERDICT command-exit 0downstream consumers (prefix form), the direction a contract widening lands in
node scripts/check-i18n-bundles.mjs --write exit 0; only service-messaging's four bundles moved

The rebuilt dist/index.d.ts carries the widened seam (readonly actorId?: string at both Notification and the delivery payload), so the consumer build above read the new declaration and not a cached one.

zh-CN / ja-JP / es-ES values for the new field are hand-written, matching the wording already used for sys_notification.actor_id in platform-objects; the generator's merge pass fills a gap with the English source, which is a fill, not a translation.

All of it re-run at the final head 76be9861f, after the last commit.

Gates. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 63 families from the 15 changed paths; all 63 were run with $? captured before any pipe, and --ran (with :: exit <code> on every line) reconciles: 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN, a DERIVED zero. Three answered exit 3 = PREREQUISITE NOT MET first (check:i18n, check:dual-build-cjs-loads, check:type-check-debt); each was re-run to a real verdict after building the closure it names, and ⛔ none is reported as a pass on its exit 3.

Repo-wide scans (pnpm lint and the wide-population families) are CI's run, not this card's.


🤖 Generated with Claude Code

https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj


Generated by Claude Code

…ng end to end from emit()

The actor stopped at sys_notification.actor_id, an object the default
permission sets grant members no read on, so an inbox row could not answer
'did I cause this?'. Widen the channel seam, snapshot the actor onto the
delivery row beside the rendered content, read it back in the dispatcher, and
materialize it on the row. Digest rows keep it null by construction.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 14 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via sys_inbox_message (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects), 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 actorId — its route source's handler names it))
  • content/docs/automation/approvals.mdx (via actorId (symbol, a field of interface DeliveryPayload), 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 actorId — its route source's handler names it))
  • content/docs/automation/flows.mdx (via /approvals/requests/:id (route, bridged from symbol actorId — its route source's handler names it))
  • content/docs/kernel/services-checklist.mdx (via /approvals/requests/:id (route, bridged from symbol actorId — its route source's handler names it))

3 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 getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/releases/v17/17-1.mdx (via sys_inbox_message (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))

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
  • 1 anchor(s) matched too much of the corpus to be a work list: sys_user (literal, 35 pages)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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 — 5 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 e0e4a5669a90555a4fc411587c056f1c9272af09packageMentionDocs.

Which tree this was computed on

This run read content/docs from 43bec809e8dbb0c6c32a5901b90aaabc1f3091df — the merge of head 76be9861fb8544283d077049a5d809b975d0555d into base e0e4a5669a90555a4fc411587c056f1c9272af09, 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 43bec809e8dbb0c6c32a5901b90aaabc1f3091df && git checkout 43bec809e8dbb0c6c32a5901b90aaabc1f3091df
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e0e4a5669a90555a4fc411587c056f1c9272af09 76be9861fb8544283d077049a5d809b975d0555d && git checkout -B drift-repro e0e4a5669a90555a4fc411587c056f1c9272af09 && git merge --no-ff 76be9861fb8544283d077049a5d809b975d0555d

node scripts/docs-audit/affected-docs.mjs --json e0e4a5669a90555a4fc411587c056f1c9272af09

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

…translating actor_id

The three translated-locale leaves for the new field are hand-written (AGENTS.md:
translated-locale VALUES are hand-written; adding or dropping KEYS is drift, and
the keys came from the writer). Re-running the writer then drops their
stale-fill provenance entries, which is the machine-checkable signal that they
are translations rather than source fills.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Contract review

PR #17886 · card #16974 · head 76be9861fb8544283d077049a5d809b975d0555d (head re-read at posting; unchanged).

Clause-② review, in-seat at the default judgement tier plus gates — the prescribed form for a non-spec lane. ⛔ Not an independent contract-review-tier review.

① Derived judgments — every accept-set / public-surface change, named and judged

# change judgement
1 Notification.actorId?: string (channel.ts) Correct. Additive and optional on the per-recipient unit every channel implementation consumes; absence = today's behaviour. This is the ruled seam (item 1).
2 DeliveryPayload.actorId?: string (outbox.ts) Correct, and the better of the two available shapes. The type carries an index signature, so the key would have been accepted untyped; declaring it makes the contract legible instead of implicit. ⚠️ outbox.ts is a sixth file beyond the ruling's five — the dev flagged this rather than absorbing it. Judged inside the authorised widening: item 4 says in as many words 「a stored JSON column gains a key」, and this IS that key.
3 sys_inbox_message.actor_id — a sys_user lookup (objects/inbox-message.object.ts) Correct. The ruled column, same semantics as sys_notification.actor_id, with item 2's digest-null fact stated in the describe text as required.

Measured on this head, ⛔ not taken from the report:

  • The published surface really is reachedservice-messaging's exports map is exactly ['.'] and index.ts re-exports these modules (23 hits). ⇒ Notification and DeliveryPayload are consumer-reachable, so Clause-②: yes is the right declaration. ⭐ Contrast with sibling PR fix(plugin-auth)!: an anonymous get-session is refused with the declared 401 envelope, not answered 200 null #17881 reviewed an hour ago, which added four exports and is nonetheless no because its index.ts is untouched and no wildcard exists. Same lane, same round, opposite verdicts, one measurement apart: reachable from the published entry is the test, ⛔ not the presence of the word export.
  • No NEW exported symbol — the interfaces already existed and gain members.
  • packages/spec touched 0 times ⇒ the lane red line is not approached, and no spec-seat card is owed. The dev's deliberate non-change is what keeps it that way: listInbox / InboxNotificationView still do not carry the actor, per the ruling's own reading that objectui reads the rows directly.
  • scripts/engine-double-contract.pinned.json touched 0 times ⇒ ⛔ this PR is NOT a baton in the ledger relay and lands on its own schedule. Deliberate per the report: new tests reused existing files and their fake-engine helper, so no ledger row was created.

② Semver

.changeset/16974-inbox-message-actor-id.md declares @objectstack/service-messaging: minor. Consistent and correct — three additive optional members, nothing removed, renamed or narrowed; patch would under-state a published-surface addition and the repo's launch-window guard forbids major outright. The changeset states the operational consequence and the end-to-end path in prose.

③ Boundary flags — each answered

  • open_questions: empty.
  • Translated-locale values hand-written — the dev declined my rework instruction, citing AGENTS.md, and it was right: the guardrail forbids hand-editing the structure while stating translated-locale values are hand-written. Ruled in full at 5647752116; ⛔ nothing reverted, and my instruction was the error.
  • Item 3 (default-permission-sets.ts) not touchedaccepted as a measured no-op: that file declares zero field-level permissions and field permissions are a subtract-only mask, so the object-level grant plus the sys_inbox_message_self RLS policy already carry the new column to the recipient. ⭐ Leaving it alone on measurement is right; editing it to satisfy the ruling's letter would have been the error.
  • Payload-shadowing noteactorId sits after the payload spread, so a top-level EmitInput.actorId wins over a same-named payload key. Accepted, ⛔ not a new hazard class: identical positioning to title, body, severity and actionUrl, and no shipped producer puts actorId in a payload. Recorded by the dev with a named carrier ([Decision] service-messaging: should fan-out consult per-tenant channel availability (a new MessagingChannel member) so no delivery row is written for a channel with no transport? — split from #17611 #17732 edits this same function).
  • Gates: 63 derived, 63 run, 0 NOT-MEASURED — a derived zero. ⭐ The three families that answered exit 3 were each re-run to a real verdict after building exactly the closure each names. That falsified my own correction 147 and is amended at 5647755594.
  • Ablation, both legs, from the committed tree under a restoring trap, restoration proven by blob hash rather than an exit code. ⭐ Worth keeping: under leg 1 the digest-absence assertion stayed green, which is why it ships with a same-tick presence control rather than alone — negative space is not coverage.

Independence pair

Implemented-by: claude/issue-16974-inbox-actor-id
Reviewed-by: session_01URLHobLUJB9K1ABV6ofdjj

⚠️ mode:subagent of this same session, so the implementer is recorded as the branch — the designed discriminator when session ids collide. Self-review by design for a non-spec lane; ⛔ not presented as independent.

Verdict: PASS on 76be9861fb8544283d077049a5d809b975d0555d

⛔ Carriers stay hung until landing, and landing waits on the last check: at posting this head reads 28 success · 5 skipped · 1 still running · 0 red, legacy success. Landing pre-check ③ is 「every check green」, and a check in progress is an honest reading, ⛔ not a pass. When it converges: clear BOTH carriers in one stroke with a provenance comment citing this record and this head, then ready → auto-merge.


Generated by Claude Code

@claude

claude Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Contract-review carriers CLEARED — provenance

domain:services execution seat, session_01URLHobLUJB9K1ABV6ofdjj, 2026-09-12T18:31Z.

⚠️ This PR is NOT a baton in the engine-double-contract.pinned.json relay and does not wait on PR #17881. Measured, ⛔ not assumed: the diff touches that ledger 0 times, deliberately — the new tests went into files that already existed and reused their fake-engine helper, so no ledger row was created.

⇒ Flipping to ready and arming auto-merge. ⛔ This seat does not approve and does not merge; the queue is the only sanctioned landing path. Tracked to MERGED, ⛔ not to 「enqueued」.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 12, 2026 18:31
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 12, 2026
Merged via the queue into main with commit a370073 Sep 12, 2026
50 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16974-inbox-actor-id branch September 12, 2026 18:59
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

messaging: sys_inbox_message 不带 actor,消费端无法识别「自己触发的回执」

2 participants