Skip to content

fix(metadata)!: sys_metadata_history.recorded_by 存 NULL 而非哨兵字符串 'system' (#4556) - #4742

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-4556-recorded-by-nullable
Aug 3, 2026
Merged

fix(metadata)!: sys_metadata_history.recorded_by 存 NULL 而非哨兵字符串 'system' (#4556)#4742
os-zhuang merged 3 commits into
mainfrom
claude/issue-4556-recorded-by-nullable

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #4556

按维护者 2026-08-02 的裁决（issue 评论 5156972029）落地：改写入路径，不放宽声明类型的语义。recorded_by 仍是 lookup('sys_user')，无 actor 的写入存 NULL，不再存哨兵字符串 'system'。⛔ 不引入「系统用户」魔法账号，⛔ 不加 actor-kind 字段。

为什么这是「改写入面」而不是「改声明」

recorded_by 声明为外键，写入却是 actor ?? 'system' —— 落库的是字符串 'system'，不是任何 sys_user 行的 id（SystemUserId.SYSTEM = 'usr_system' 在新运行时下也不再自动供给，写成它同样解析不到）。任何按声明来读这个字段的消费者（expand、报表的 owner 列、审计时间线的「谁改的」）都拿到一个解析不出来的 id。NULL 是「无链接」的标准表达，也正是这一列 deleteBehavior: 'set_null' 已有的含义。

落地清单

位置 改动
metadata-core/src/objects/sys-metadata-history.object.ts recorded_by 加 description，写明「NULL = 系统发起（boot 同步 / 迁移 / 定时任务）」；声明与实际值就此一致
metadata-core/src/types.ts PutOptions.actor / DeleteOptions.actor 放宽为 string | null 且保持 required；MetadataEvent.actor、MetadataItem.authoredBy 变 nullable
metadata-protocol/src/sys-metadata-repository.ts 两处写入 opts.actor ?? null；三处读路径不再把缺失渲染成 'unknown'；close() 的合成排空事件不再伪造 actor
metadata-protocol/src/protocol.ts 8 处 ?? 'system' 候选里，只改真正流向 recorded_by 的 5 处
metadata-protocol/src/migrations/recorded-by-sentinel.ts（新） 存量 'system' → NULL 的 ADR-0119 D2 migration plan
cli/src/commands/migrate/recorded-by.ts（新） os migrate recorded-by，默认 dry run，--apply 才写
platform-objects/src/apps/translations/*.objects.generated.ts 上面那条 description 进 i18n schema 成为该字段的 help，四个 locale bundle 随之再生（见「i18n 再生」一节）

actor 保持 required 而不是变 optional，是刻意的。 变 optional 的话，一个忘记传 actor 的调用点会静默地变成「系统写入」；保持 required 但可为 null，编译器强迫每个调用点明确表态是哪一种。这条对 AI 生成的代码尤其要紧 —— 少写一个字段不会再变成一个假外键。

protocol.ts 那 8 处：逐处判断的结论

PM 抓出的是按 ?? 'system' 形状匹配的候选集，不是全都写 recorded_by。改动 5 处，保留 3 处：

  • 改（流向 repo.put / promoteDraft / restoreVersion / delete → recorded_by）：saveMetaItem、publishMetaItem、revertCommit、rollbackMetaItem、deleteMetaItem。
  • 不改 sys_metadata_audit 的两处（读 + 写）：那一列是 Field.text，其 description 原文就是 "Acting principal — user id, system id, or 'system'"。声明与实际一致，'system' 在那里是诚实的，改它反而制造新的不一致。
  • 不改 PublishMaterializer.actor：那是插件回调的入参（args.actor ?? 'system'），不落任何 lookup 列；目前唯一注册的 materializer（plugin-security）根本没读它。改错比漏改更糟，所以留给它自己的单子。

存量迁移：走 migration journal，不手写 boot backfill

os migrate recorded-by 通过 ADR-0119 D2 的 runMigrationJournal（#4617 / PR #4668）执行，因此天然拿到 chunk 级事务、chunk_done 写在事务内、崩溃后 os migrate resume 可续。plan 本体放在写出哨兵的那个包里（metadata-protocol），CLI 只是操作者意图的入口 —— 与「boot 负责发现、CLI 负责动作」的既有分工一致。

  • 默认 dry run（Schema sync is additive-only: non-additive metadata changes (required→optional, type, drop, rename) silently diverge from existing DBs; need drift detection + os migrate #2186：os migrate 的子命令不得意外改库）。
  • 重跑幂等：load() 只选仍带哨兵的行，所以第二次 --apply 选到 0 行、提交 0 个 chunk、什么都不改 —— 有测试钉住。
  • 语义等价：这一列历史上只存过这一个哨兵，由唯一一个表达式 actor ?? 'system' 写入，两种写法都表示「无 actor」，只有 NULL 在声明类型里表达得出来。changeset 里写明了这一点。
  • 写入带事务上下文：ctx.context 同时携带 isSystem —— 这正是 ObjectQL 允许写 readonly 列的条件。漏掉它的话迁移会静默地什么都不做却报成功，所以专门有一条测试断言每次 update 的 context 是 { __tx: true, isSystem: true }。

测试

i18n 再生（第二个 commit）

首轮 CI 的 TypeScript Type Check 红了，失败点不是 tsc 而是同 job 里的 check-i18n-bundles：字段的 description 会进 i18n schema 成为该字段的 help，所以 platform-objects 的四个 locale bundle 变陈旧。

按工具自述的修法处理：node scripts/check-i18n-bundles.mjs --write（merge 模式，既有翻译一律保留）。diff 恰好是每个 locale 新增一个 help key，无任何无关漂移。非英文 locale 里填的是源文，这是该工具「待翻译」的设计语义，不是翻译主张。

复验：node scripts/check-i18n-bundles.mjs → OK（9 个包全部 in sync）；pnpm check:i18n-coverage → OK（12 个 config，660 条已入基线，none new）。

（顺带记录一个 AGENTS.md §9 的现场：check:i18n-coverage 一开始报的是 Cannot find module .../connector-mcp/dist/index.mjs，那是 worktree 构建状态陈旧，不是本 PR 的问题 —— 把 plugins / connectors 构建齐之后即绿。）

关于 #4441 / #4551 两处豁免的判断（本单不动，已开 #4743）

按 PM 约束，objectql/src/engine.ts 的 readonly 收窄与 dangling-reference-audit.ts 的巡检跳过本 PR 一行未动。我的判断：

  • data: a lookup accepts an id that does not exist in the referenced object — including the RBAC permission-set link tables #4441 的写入面收窄应当保留。 它的成立理由不是「recorded_by 里有哨兵」，而是「只有调用方提供的值才由调用方负责」—— 非系统调用方写 readonly 字段的值在写入前就被 stripReadonlyFields / stripReadonlyForInsert 剥掉了，留下的一定是平台自己写的。这个论证与本 PR 无关，独立成立。但那段注释现在会误导人：它把一个已经不存在的 wart 当作收窄的理由记在那里，下一个读到的人会以为收窄是个可以拆掉的临时补丁。
  • isSystem 写入仍可产生悬空 lookup 引用——需要一条只报告不拦截的巡检（#4441 残留） #4551 的巡检跳过反而值得重新评估。 它自己的文档承认，剔掉 recorded_by 之后，被豁免的 readonly 引用字段就只剩 created_by / updated_by / organization_id —— 这些是货真价实的 id，而且真的会悬空（删掉一个用户，他创建过的每一行 created_by 就指向不存在的行）。那恰恰是巡检本该报告的一类。收窄这个跳过是有实际价值的，但影响面不小（删过用户的库会瞬间亮起大片），需要单独决策。

两条已记入 #4743（未认领，含 A/B/C 三个方向与倾向性建议）。

边界

验证输出

metadata-core        Test Files  8 passed (8)      Tests  103 passed (103)
metadata-protocol    Test Files 30 passed (30)     Tests  246 passed (246)
objectql             Test Files 104 passed (104)   Tests 1643 passed (1643)
core                 Test Files 28 passed (28)     Tests  458 passed (458)
metadata             Test Files 14 passed (14)     Tests  290 passed (290)
runtime              Test Files 80 passed (80)     Tests 1092 passed (1092)
rest                 Test Files 37 passed (37)     Tests  559 passed (559)
cli                  Test Files 65 passed (65)     Tests  581 passed (581)

pnpm --filter @objectstack/metadata-core typecheck   → tsc --noEmit, clean
pnpm --filter @objectstack/cli typecheck             → tsc --noEmit, clean
node scripts/check-type-check-coverage.mjs           → OK, 60/77 covered（未新增 debt）
node scripts/check-i18n-bundles.mjs                  → OK, 9 packages all in sync
pnpm check:i18n-coverage                             → OK, 12 configs, none new
eslint（全部新增文件）                                 → clean

（metadata-protocol 在 #4311 的 DEBT 台账里、没有 typecheck 脚本，所以单独跑了 tsc --noEmit -p：59 条报错全部落在既有的 *.test.ts 债务文件里，protocol.ts 与 sys-metadata-repository.ts 零报错。）

第一轮远端 CI：除 TypeScript Type Check 外全绿（Test Core、Dogfood Regression Gate 1/2 + 2/2、Dogfood Verify CLI、Build Core、Temporal Conformance、ESLint、Check Changeset、Check PR Size、重复认领闸门）。TypeScript Type Check 的红由上面的 i18n 再生 commit 修复。

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

claude added 2 commits August 3, 2026 01:19
@claude
fix(metadata)!: store NULL in sys_metadata_history.recorded_by, not t…
5a577ea 
…he 'system' sentinel (#4556)

`recorded_by` is declared `Field.lookup('sys_user', { readonly: true })` — a
foreign key — while the write path filled it with `actor ?? 'system'`. Every
actor-less metadata write therefore stored the STRING `'system'` in a column
whose declared type says "an id of a sys_user row", and no such row exists.
Declared != actual, at the data layer.

Per the maintainer's 2026-08-02 ruling, the fix is on the WRITE path, not the
declaration: `recorded_by` stays a lookup, an actor-less write stores NULL, and
NULL means "system-initiated (boot sync, migration, scheduled job)". No magic
system-user account, no actor-kind companion column.

- `sys-metadata-history.object.ts`: `recorded_by` carries a `description`
  stating the NULL semantics, so the declaration says what the column holds.
- `metadata-core/types.ts`: `PutOptions.actor` / `DeleteOptions.actor` widen to
  `string | null` and stay REQUIRED, so every call site must say which of the
  two it is; `MetadataEvent.actor` and `MetadataItem.authoredBy` become
  nullable.
- `sys-metadata-repository.ts`: both history writes store `opts.actor ?? null`;
  the three read paths surface `null` instead of inventing `'unknown'`;
  `close()`'s synthetic drain event carries no actor.
- `protocol.ts`: the five `?? 'system'` sites that flow into `recorded_by`
  (save / publish / revert-commit / rollback / delete) pass `null`. The three
  that do NOT are left alone: `sys_metadata_audit.actor` is a `text` column
  whose declaration already admits `'system'`, and `PublishMaterializer.actor`
  is a plugin callback argument that reaches no lookup column.
- New `os migrate recorded-by` rewrites stored `'system'` to NULL through the
  ADR-0119 D2 migration journal (chunk-atomic, resumable, dry run by default,
  idempotent on re-run). The plan itself lives with the code that wrote the
  sentinel, in `metadata-protocol/src/migrations/`.

Tests: repository-level NULL round-trip, the plan under the real journal runner
(idempotence, chunking, compensation, transaction binding), and an end-to-end
protocol suite against a real ObjectQL engine where `recorded_by` is declared
as the real readonly lookup — which also pins that create/publish/delete
authoring still passes the #4441 integrity check.

Deliberately NOT touched: #4441's `readonly` narrowing in `objectql/engine.ts`
and #4551's audit skip. See the PR description.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@claude
Merge remote-tracking branch 'origin/main' into claude/issue-4556-rec…
cfb22aa 
…orded-by-nullable
@vercel

vercel Bot commented Aug 3, 2026 •
edited
Loading

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 Aug 3, 2026 1:36am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/cli, @objectstack/metadata-core, @objectstack/metadata-protocol, @objectstack/platform-objects.

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

  • content/docs/ai/skills-reference.mdx (via packages/cli)
  • content/docs/api/client-sdk.mdx (via @objectstack/cli)
  • content/docs/api/data-flow.mdx (via @objectstack/cli)
  • content/docs/api/environment-routing.mdx (via @objectstack/cli)
  • content/docs/api/error-catalog.mdx (via @objectstack/cli)
  • content/docs/automation/hook-bodies.mdx (via packages/cli)
  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-core, @objectstack/metadata-protocol)
  • content/docs/deployment/backup-restore.mdx (via @objectstack/cli)
  • content/docs/deployment/cli.mdx (via @objectstack/cli)
  • content/docs/deployment/self-hosting.mdx (via @objectstack/cli)
  • content/docs/deployment/validating-metadata.mdx (via packages/cli)
  • content/docs/getting-started/your-first-project.mdx (via @objectstack/cli)
  • content/docs/kernel/runtime-services/data-service.mdx (via packages/cli)
  • content/docs/kernel/runtime-services/index.mdx (via packages/cli)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/permissions/authentication.mdx (via @objectstack/cli)
  • content/docs/plugins/index.mdx (via @objectstack/cli)
  • content/docs/plugins/packages.mdx (via @objectstack/cli, @objectstack/metadata-core, @objectstack/platform-objects)
  • content/docs/protocol/kernel/plugin-spec.mdx (via @objectstack/cli)
  • content/docs/protocol/kernel/realtime-protocol.mdx (via @objectstack/cli)
  • content/docs/releases/implementation-status.mdx (via @objectstack/cli)
  • content/docs/releases/v12.mdx (via @objectstack/metadata-core)
  • content/docs/releases/v16.mdx (via @objectstack/cli)
  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)
  • content/docs/ui/setup-app.mdx (via @objectstack/platform-objects)

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.

@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Open
@claude
chore(i18n): regenerate platform-objects bundles for the new recorded…
f794337 
…_by help text (#4556)

The `description` added to `sys_metadata_history.recorded_by` lands in the
i18n schema as that field's `help` string, which made all four
platform-objects locale bundles stale and turned `check-i18n-bundles` (in the
required `TypeScript Type Check` job) red.

Regenerated with `node scripts/check-i18n-bundles.mjs --write` — merge mode, so
every existing translation is preserved. The diff is exactly one added `help`
key per locale and nothing else; the non-English locales carry the source
string, which is this tool's "awaiting translation" state, not a translation
claim.

Verified after: `node scripts/check-i18n-bundles.mjs` → OK (9 packages, all
bundles in sync); `pnpm check:i18n-coverage` → OK (12 configs, none new).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
@os-zhuang Claude

os-zhuang commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

真实 CLI 跑通记录（不是单测，是 os migrate recorded-by 打真库）

单测证明的是 plan 在 runner 上的行为；下面是把命令接到一个真实 booted stack + 真实 sqlite 文件上跑出来的结果，因为「迁移能不能真的改到那一列」是一个单测替身回答不了的问题 —— recorded_by 是 readonly，写不进去的话迁移会静默地什么都不做却报成功。

准备：用 examples/app-crm 起一次 os migrate recorded-by（OS_DATABASE_URL=file:...t.db）建库，然后直接往 sys_metadata_history 塞三行历史数据 —— 两行哨兵、一行真 actor：

seeded: [{"id":"dog_h1","recorded_by":"system"},
         {"id":"dog_h2","recorded_by":"usr_real"},
         {"id":"dog_h3","recorded_by":"system"}]

1. dry run 认出待改行，且不写库

{ "planId": "metadata.recorded-by-sentinel-to-null", "sentinel": "system", "pending": 2, "applied": false }

2. --apply --yes 走 journal 完成

{ "runId": "bed40c58-9cc6-4ed1-98b7-fc3e8bc1c7be", "status": "completed",
  "chunksTotal": 1, "chunksCommitted": 1, "chunksCompensated": 0,
  "planHash": "d3139f2c3baa137a6c66eb3190c7ba96", "pending": 2, "applied": true }

3. 直接查 sqlite 验证 —— 关键在 typeof()

history rows: [{"id":"dog_h1","recorded_by":null,"t":"null"},
               {"id":"dog_h2","recorded_by":"usr_real","t":"text"},
               {"id":"dog_h3","recorded_by":null,"t":"null"}]

typeof(recorded_by) = 'null' 是 SQLite 自己的判定：落的是 SQL NULL，不是字符串 'null'、不是空串、不是 'system'。真 actor 那行原封不动。这条正是验收里「无 actor 的元数据写入落库为 NULL，不是任何字符串」。

4. journal 留痕

journal: [{"kind":"run_started","seq":0},
          {"kind":"chunk_started","chunk_index":0,"seq":1},
          {"kind":"chunk_done","chunk_index":0,"seq":2},
          {"kind":"run_done","seq":3}]

5. 重跑幂等

{ "planId": "metadata.recorded-by-sentinel-to-null", "pending": 0, "applied": true,
  "status": "completed", "chunksCommitted": 0 }

再查一次库：三行完全不变，journal 也没有新增事件。第二次连 runner 都没进 —— pending === 0 时 CLI 直接短路返回，所以一次空跑不会在 journal 里留下噪音行。

6. 命令可发现

os migrate recorded-by --help 正常输出（oclif 自动发现，无需手工注册表）。

顺带记录一个与本 PR 无关的现象，避免下一个人误判：命令返回 JSON 之后，进程关停时的 dangling-reference 巡检会打出 Unable to acquire a connection（sys_metadata / sys_view_definition 列为 unreadableObjects）。那是 stack shutdown 与巡检的时序问题，在 os migrate resume 等既有子命令上同样存在，与本改动无关，也没有影响命令的返回值与退出码。没有顺手改（Prime Directive #10 的边界），如需要我可以另开单。

Generated by Claude Code

Generated by Claude Code

@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
@os-zhuang
os-zhuang marked this pull request as ready for review August 3, 2026 01:50
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
Merged via the queue into main with commit 65f184b Aug 3, 2026
22 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4556-recorded-by-nullable branch August 3, 2026 01:59
This was referenced Aug 3, 2026
Merged
Merged
Closed
Merged
@xuyushun441-sys xuyushun441-sys mentioned this pull request Aug 3, 2026
Open
@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sys_metadata_history.recorded_by 是 lookup('sys_user') 却存哨兵字符串 'system'——声明的类型与实际存的值不是一回事

2 participants

@os-zhuang @claude