Skip to content

fix(service-messaging,plugin-audit): the service that writes sys_notification declares it (#4154) - #4182

Merged
os-zhuang merged 2 commits into
mainfrom
claude/inmemory-to-sqlite-memory-5f7n9e
Jul 30, 2026
Merged

fix(service-messaging,plugin-audit): the service that writes sys_notification declares it (#4154)#4182
os-zhuang merged 2 commits into
mainfrom
claude/inmemory-to-sqlite-memory-5f7n9e

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #4154

比 issue 里写的更糟一点

我立 #4154 时说的是「MessagingServicePluginsys_notification 但不声明它,真实 boot 能跑只因 platform-objects 恰好也加载」。查下去发现两处需要更正,而且实际情况更严重

  1. 注册它的不是「platform-objects 恰好加载」,是 AuditPlugin —— audit-plugin.ts 把它放进了自己的 manifest objects
  2. AuditPlugin 在 CLI 里是 optional 的serve.ts:1778 "Pair: AuditPlugin — optional")。

所以这不是「层次不好看」,是装了 messaging 不装 audit,每次 notify() 都会炸 no such table: sys_notification —— 而 emit() 是整条 ADR-0030 pipeline 的唯一入口

修法是代码自己预约好的

audit-plugin.ts 原注释:

sys_notification is contributed here but owned by platform-objects; it is being reworked by ADR-0030 messaging (notification→event), so it stays put until that migration lands.

ADR-0030 早已落地(MessagingServicePlugin 带完整 L2/L4/L5 pipeline、对象 docstring 写着 "Re-modeled (ADR-0030)"、migrate-sys-notification-to-event.ts 也在)。这条寄放到期了,把它挪到位即可 —— 不是新发明的方案。

佐证 AuditPlugin 本就不该持有它:

  • 从不直接写这行 —— 走 messaging 的 emit() 入口(audit-writers.ts:41getMessaging()
  • 它自己的排除清单里就标着 'sys_notification', // messaging-owned (ADR-0030); its own lifecycleaudit-writers.ts:112

改了什么

  • MessagingServicePlugin 的 manifest objects 加上 SysNotification,并加进 provisionSystemTables(表随它统领的 pipeline 一起建,而不是等第一次写时懒建)
  • AuditPlugin 的 manifest 去掉它,陈旧的寄放注释换成说明去向
  • service-messaging 增加 @objectstack/platform-objects 依赖(无环:→ metadata-core/spec,没有回边)

定义归属没变:对象仍在 @objectstack/platform-objects、仍在 PLATFORM_OBJECTS_BY_PACKAGE 里。拥有定义 ≠ 把它贡献进运行中的 kernel,这两件事本来就分开。这也正是仓库既有模式:service-job import SysJob/SysJobRunservice-queue import SysJobQueuerest import SysImportJob —— 拥有行为的 service 负责贡献

验证

最有说服力的一条:删掉 #4100 里为这个缺陷加的 fixture 绕行

notifications.hono.integration.test.ts 当时必须自己 AppPlugin({ objects: [SysNotification] }) 才能启动——那正是这个部署缺陷的缩影。现在它只引导 messaging 就能跑通,绕行代码已删除。

另加一条直接的回归断言(plugin-declares-event-object.test.ts),把不变量钉在插件本身,而不是等它在三层之外表现为 404 或 Find operation failed已验证它在未声明的 manifest 上会红,不是空洞断言。

  • 含 dogfood 的全量强制重跑:132/132 全绿
  • pnpm run lint 干净

关联


Generated by Claude Code

…fication declares it (#4154)

`MessagingService.emit()` writes `sys_notification` on every call — it is the
pipeline's single ingress (ADR-0030 L2). But the object was contributed to the
manifest by AuditPlugin, parked there with a comment saying it would stay
"until that [ADR-0030] migration lands". The migration landed; the parking did
not move.

That left a real deployment hole, because AuditPlugin is an OPTIONAL pair in the
CLI's plugin table. Install messaging without audit and nothing registers the
object, so the engine has no schema to issue DDL from and every notify() fails
with `no such table: sys_notification`. AuditPlugin never wrote the row itself —
it deliberately routes through this service's emit() ingress (`getMessaging()`
in audit-writers.ts), and its own exclusion list already annotates the object as
"messaging-owned (ADR-0030)".

The contribution now lives with the writer, matching how every other
service-owned platform object is handled here: service-job imports
SysJob/SysJobRun, service-queue imports SysJobQueue, rest imports SysImportJob.
Ownership of the DEFINITION is unchanged — the object stays in
@objectstack/platform-objects and in PLATFORM_OBJECTS_BY_PACKAGE, because owning
a definition and contributing it to a running kernel are different things. It is
also added to provisionSystemTables so the table is created with the rest of the
pipeline it heads rather than lazily on first write.

Found while migrating notifications.hono.integration.test.ts to in-memory SQLite
in #4065: that suite had to register the object itself to boot, which was the
deployment bug in miniature. The workaround is deleted here — the suite now
boots messaging alone and passes, which is the proof.

A direct regression test pins the invariant at the plugin rather than three
layers downstream, where it surfaced as a 404 or a `Find operation failed`.
Verified it fails against the un-declared manifest before landing the fix.

Verified: full forced suite including dogfood green (132/132), lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L5q3BZMpdiAueHf2emhDY9
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 30, 2026 2:25pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-audit, packages/services.

9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx (via packages/services)
  • content/docs/deployment/cli.mdx (via @objectstack/plugin-audit)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-audit)
  • content/docs/kernel/runtime-services/audit-service.mdx (via packages/services)
  • content/docs/kernel/runtime-services/index.mdx (via packages/services)
  • content/docs/kernel/runtime-services/settings-service.mdx (via packages/services)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-audit, packages/services)
  • content/docs/protocol/kernel/i18n-standard.mdx (via packages/services)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-audit)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file tests tooling labels Jul 30, 2026
… answer now (#4154)

`objects/index.ts` listed `sys_notification` under "Intentionally NOT moved
here" with the reason "reworked by ADR-0030 messaging" — a pending state. The
rework landed and the contribution moved to service-messaging, so the note now
says where it went and why the definition still lives in platform-objects.

Found by the docs-drift check on #4182 flagging plugin-audit. The nine docs it
listed were all verified accurate and need no change — `audit-service.mdx` only
references sys_audit_log/sys_activity, which this plugin still registers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L5q3BZMpdiAueHf2emhDY9
@os-zhuang
os-zhuang marked this pull request as ready for review July 30, 2026 14:38
@os-zhuang
os-zhuang merged commit a8dcc37 into main Jul 30, 2026
18 checks passed
@os-zhuang
os-zhuang deleted the claude/inmemory-to-sqlite-memory-5f7n9e branch July 30, 2026 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MessagingServicePlugin 写 sys_notification 但不声明它——靠 platform-objects 恰好也加载才能工作

2 participants