Skip to content

fix(objectql): make transaction() 的两条既声明 caveat 可被观测 (#4619) - #5724

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-4619-transaction-observability
Aug 6, 2026
Merged

fix(objectql): make transaction() 的两条既声明 caveat 可被观测 (#4619)#5724
os-zhuang merged 1 commit into
mainfrom
claude/issue-4619-transaction-observability

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Part of #4619

⛔ 刻意不写 Fixes —— 本 PR 只做 #4619 的 engine 侧「可发现性」半边,契约收紧的半边未做,issue 不应随本 PR 关闭。

前提复核(先于实现)

issue 正文给的行号 engine.ts:4934-4973 已漂移,但两个缺陷在 origin/main依然成立,按内容定位:

  • 静默降级:if (!drv?.beginTransaction) { return callback(baseContext); }
  • default-driver-only:this.defaultDriver ? this.drivers.get(this.defaultDriver) : undefined

契约声明在 packages/spec/src/contracts/objectql-engine.ts,TSDoc 明写这两条 caveat 是声明语义的一部分(ADR-0119 D1)。所以声明了 ≠ 修好了:今天的问题不是行为错,而是行为完全没声音

本 PR 做了什么

行为一字未改。只加两处诊断 + 一处上下文标记:

级别 频次 触发
warnTransactionUnsupported warn 每引擎实例每驱动一次 驱动无 beginTransaction,callback 将在无事务无回滚下运行
reportWriteOutsideTransaction error 每事务每数据源一次 事务内的 insert/update/delete 解析到该事务覆盖不到的驱动

日志级别不是随手定的,按 AGENTS.md「Degradation log levels」的判据问一次:

两条消息都按该节要求同时给出后果修法

TransactionScope 记在 ambient txStore 条目上,只记录「这个事务属于哪个 driver」,不参与任何路由

引擎自身的 transaction() 与沙箱侧的 ScopedContext.transaction()(即 hook/action body 里的 ctx.api.transaction)是同一缺陷的两份实现,现在共用同一对 engine 侧 helper,所以两个入口共享一份「只说一次」的额度,措辞也不会各自漂移。

刻意没做

opts.require 抛错、跨驱动拒绝、owned-vs-joined 信号进签名 —— 三者都改 packages/spec 的声明语义,归 spec 车道,PM 另立。

验证

  • pnpm --filter @objectstack/objectql test:122 文件 / 1984 测试全绿(含既有 engine-ambient-transaction.test.ts 的 ADR-0067 D2 用例,全部承重保留)
  • pnpm --filter @objectstack/objectql typecheck:干净
  • 消费半径(该规则的调用方,不是被改包):metadata 495、metadata-protocol 407、runtime 1436、driver-sql 860 —— 全绿
  • 门:check:durability-log-level(24 seam 全响)、check:query-options-erasure(84 site,无新增)、check:nul-bytes 通过 + 控制字符自查越过门的盲区

反向验证(方向先判后跑):预判为红向 —— 摘掉接线后诊断计数由 1 → 0,断言「恰好一条」的用例应转红。实测 14 例中 10 例转红,与预判一致。

剩下 4 例保持绿,这是正确且预期的:它们是断言「不出声」的反向对照(默认数据源内的写、事务外的映射写、事务关闭后的写、驱动支持事务时),摘掉诊断当然不会让它们变红。为确认这 4 例不是幻影检查,另跑一次似是而非实现探针(不判断是否处于事务、只要不是默认驱动就报),预判其中 2 条反向对照应转红 —— 实测正是这 2 条(外加 1 条去重断言被连带打破)。

#5351 的关系(重要)

测试中实测到:ledger_db 的 driver 收到的是 primary 的事务句柄 —— 即 ambient 句柄跨数据源泄漏。这不是本 PR 引入的(未触碰 buildDriverOptions),且已有 #5351 立单在先,故按 objectstack#4949 的「先搜再立、命中则评论」规则,不另立重单,已在 #5351 下评论。

该行为在新测试里被如实钉住(toEqual({ __trx: 'primary' })),标注为「观测到,非认可」—— 这样修 #5351 的人会看到这条测试,而不是让它在一个当初只是猜测的 toBeUndefined() 底下悄悄改变。

顺带:本 PR 加的 TransactionScope.driver 恰好就是 #5351 推荐方案 A 点名需要的那处前置(「需要 txStore 记录属主 driver」)。本 PR 只报告不拦截,不预判该单的裁决。

文件面

packages/objectql/src/engine.ts(transaction() 方法区 + 三处写路径各一行诊断调用)、新增测试、changeset。

⚠️#5504 同文件:本 PR 在 insert/update/delete 的方法开头(getDriver 之后紧邻一行),距 #5504 所动的 insert/update 返回路径约 250–400 行,区域不相交。


Generated by Claude Code

…4619)

`ObjectQL.transaction()` carries two caveats that are part of its DECLARED
meaning (ADR-0119 D1, packages/spec/src/contracts/objectql-engine.ts), not
hidden behaviour: it degrades to no-transaction/no-rollback when the default
driver has no `beginTransaction`, and it covers the DEFAULT datasource only,
so an object routed elsewhere by `setDatasourceMapping` is written outside it.

Declaring them is not the same as being able to observe them, and both were
completely mute. A caller asking for atomicity and not getting it had no way
to find out; a multi-datasource "atomic" unit of work that partially committed
reported nothing — one store reverted, the other kept its rows, and the caller
saw only that the whole thing failed. Same shape as `batchData`'s `atomic`
flag being a lie for as long as it was (ADR-0119 D4).

Behaviour is unchanged. This is the observability half only:

- `warnTransactionUnsupported` — `warn`, once per driver per engine instance,
  on the degrade path. Names the driver, the consequence and the fix. `warn`
  rather than `error` per AGENTS.md's judgment question: at that moment
  nothing has been lost, a capability is simply absent, which is the
  `if (!capability)` composition branch that section keeps at `warn`.
- `reportWriteOutsideTransaction` — `error`, once per transaction per
  datasource, when an insert/update/delete inside an open transaction resolves
  to a driver that transaction does not cover. `error` because afterwards the
  system looks entirely normal from the outside while a write it claimed was
  part of an atomic unit has landed on its own: the durability class.
- `TransactionScope` on the ambient `txStore` entry records WHICH driver owns
  the open transaction. Observability record only — it routes nothing.

Both surfaces report through the same engine-side helpers, so the direct
(`engine.transaction`) and sandboxed (`ScopedContext.transaction`, reached as
`ctx.api.transaction`) paths share one budget and one wording.

Tightening either caveat — an `opts.require` that throws, refusing a
cross-driver write, or an owned-vs-joined signal in the callback signature —
changes the contract's declared semantics and is deliberately NOT done here;
that half of #4619 belongs to `packages/spec`.

Note for #5351: the `TransactionScope.driver` added here is exactly the
"txStore records the owning driver" prerequisite its recommended option A
names. This PR only reports the mismatch; it does not gate the handle.

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

vercel Bot commented Aug 6, 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 Aug 6, 2026 2:08am

Request Review

@github-actions github-actions Bot added the size/l label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

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

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/objectql)
  • content/docs/kernel/services.mdx (via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx (via @objectstack/objectql)
  • content/docs/plugins/index.mdx (via @objectstack/objectql)
  • content/docs/plugins/packages.mdx (via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx (via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)
  • content/docs/releases/implementation-status.mdx (via @objectstack/objectql)

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 tests tooling labels Aug 6, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 6, 2026 02:37
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 6, 2026
Merged via the queue into main with commit 488b66c Aug 6, 2026
24 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4619-transaction-observability branch August 6, 2026 02:50
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

Development

Successfully merging this pull request may close these issues.

2 participants