feat(automation): update_record/create_record 步骤对被静默剥离的写入字段挂 warning(#3407) - #3413
Merged
Merged
Conversation
…ings (#3407) update_record reported an unconditional success even when the data layer legally stripped the requested write fields (static readonly #2948, conditional readonlyWhen #3042) — the only trace was a server-side logger warn, invisible in the flow run trace, which is how #3356's approval stage write-backs failed behind a clean 3ms success. - spec: new DroppedFieldsEventSchema ({object, fields, reason}) in data/data-engine.zod.ts (Zod-first, JSON-representable, published to the schema manifest) + a WriteObservabilityOptions mixin (onFieldsDropped listener) on IDataEngine.insert/update option params in contracts/data-engine.ts. The listener is TS-contract-level and in-process only — a function is unrepresentable in JSON Schema and never crosses the RPC boundary. Core's mirror contract kept in sync. - objectql: engine.update() reports each strip pass's dropped keys + reason through options.onFieldsDropped at all four strip sites (single-id + bulk x readonly + readonly_when), diffing before/after key sets (exact: strips return the same reference when nothing dropped). A throwing listener never breaks the write. System-context writes skip the readonly strip and report nothing, as before. insert() accepts the option for signature symmetry but strips nothing today (INSERT is readonly-exempt; FLS write denial throws). - service-automation: NodeExecutionResult and StepLogEntry gain advisory warnings?: string[] (persisted through run history; success semantics unchanged). update_record / create_record attach one warning per strip event naming the dropped fields and expose a structured droppedFields output ({<nodeId>.droppedFields}) for downstream nodes. Design per issue #3407 direction 1 (engine structured feedback) — chosen over run-scoped logger routing (direction 3): shallower intrusion, and the node gets a structured field list instead of log text. FLS is not wired: its write gate now throws (fail-closed) instead of stripping, so it is no longer a silent-drop case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HaofCZbsPTHE2oJedvKd77
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Three rerun_failed_jobs rounds on the runs queued during the 2026-07-22 GitHub-hosted-runner outage all failed in seconds with no logs (HTTP 404), including after GitHub resolved the incident at 22:09 UTC. Fresh push to mint fresh workflow runs instead of re-running the poisoned run records. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HaofCZbsPTHE2oJedvKd77
Contributor
Author
|
CI 红灯是仓库级 Actions 故障,与本 PR 代码无关 — 诊断如下:
本地全量测试是绿的(spec 6818 / objectql 1042 / service-automation 347 / core 386)。我会继续定时自检,GitHub 恢复后自动重踢并汇报。 Generated by Claude Code |
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 116 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…docs) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HaofCZbsPTHE2oJedvKd77
os-zhuang
marked this pull request as ready for review
July 24, 2026 11:43
Contributor
Author
|
✅ CI 全绿,已转 ready for review。 GitHub Actions 恢复(7/24 ~11:20 UTC)后:
CI 已无阻塞,可以 review/merge。 Generated by Claude Code |
This was referenced Jul 24, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 24, 2026
…#3431) (#3448) Wire the engine's onFieldsDropped strip-observability channel (#3413) through the DataProtocol and REST write path. updateData collects the engine's readonly/readonlyWhen strips; createData surfaces the #3043 static-readonly ingress strip via a payload diff. Both attach an optional droppedFields to the response. spec: Update/CreateDataResponseSchema gain optional droppedFields (backward-compatible). rest: PATCH/POST echo drops as the X-ObjectStack-Dropped-Fields header and keep the structured list on the body; status/success unchanged. Deferred to a follow-up: bulk (updateManyData/createManyData/batchData) + GraphQL wiring, typed @objectstack/client warnings, CORS exposeHeaders.
This was referenced Jul 24, 2026
Closed
os-zhuang
added a commit
that referenced
this pull request
Jul 24, 2026
…design time (#3425) (#3465) A flow `update_record` node that writes a field the target object declares `readonly: true`, under the default `runAs: 'user'`, is a silent no-op: the objectql engine strips static-`readonly` fields from a non-system UPDATE payload (#2948) so the write never lands, while the step still reports success. #3407/#3413 surfaced the strip at run time; this shifts discovery left to `os validate` / `os build`. - New `@objectstack/lint` rule `validateReadonlyFlowWrites`: static readonly + literal field under runAs!=='system' → error (gates); readonlyWhen → warning. Skips create_record (INSERT is engine-exempt), runAs:'system' flows, templated object names, and non-literal fields maps to stay false-positive-free. - Wired into `os validate` and `os compile`/`os build` (mirrors the security posture gate). Verified: reds on a violating app-crm flow, clean on all example apps. - Documents the formal contract in the objectstack-data / -automation skills: readonly governs the user/API surface; system writers (runAs:'system', hooks, seeds) maintain it. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 28, 2026
baozhoutao
added a commit
that referenced
this pull request
Jul 28, 2026
…y` is enforced (#3835) `POST /batch` called `ql.insert` directly, and the engine's INSERT path is static-`readonly`-exempt by design (#3413) — the strip that stops a non-system caller from seeding a read-only column lives at the protocol's create ingress (#3043). So the same forged value was dropped on `POST /data/:object` and written through on `/batch`: one rule, two answers. Create ops now go through `p.createData`, the ingress itself, rather than a second copy of the strip at the REST layer. One ingress means a future change to its policy covers the batch for free, and the carve-outs it already encodes stay intact — the platform-object exemption (a `sys_`/`managedBy` object's own guard must REJECT a forged value, not silently swallow it) and the `isSystem` exemption. `trxCtx` is passed as the context, so the insert still joins the batch transaction and `$ref` resolution is unaffected; the ingress's `droppedFields` fold into the batch's per-op list. Update ops are untouched — the engine enforces both strips on its update path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
…cy, dropped fields, and the batch create ingress (#3794, #3835) (#3834) * fix(approvals,rest): surface the node's lock policy and the batch's dropped fields (#3794) An approval flow reported record writability wrong in both directions: what the user could change said "locked", and what they couldn't said "updated successfully". Both halves were missing signal, not wrong behaviour — the server did the right thing and told nobody. `rowFromRequest` now emits `locks_record`, read from the same `node_config_json` snapshot the `beforeUpdate` lock hook reads, with the same default-true. A client could previously only see "a request is pending" and had to guess whether that locked the record; the Console guessed "locked" every time, which hides the whole point of a `lockRecord: false` node. `POST /batch` (cross-object transactional batch) never wired `onFieldsDropped`, so the one write path the Console's master-detail record form takes was also the one path with no write-observability — a `readonlyWhen` strip there was completely silent. It now collects per-op events and returns them as a top-level `droppedFields` list tagged with each operation's index; omitted when empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(spec): regenerate the batch reference for CrossObjectBatchDroppedFields (#3794) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(spec): refresh the API-surface snapshot for CrossObjectBatchDroppedFields (#3794) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rest): route batch creates through the create ingress so `readonly` is enforced (#3835) `POST /batch` called `ql.insert` directly, and the engine's INSERT path is static-`readonly`-exempt by design (#3413) — the strip that stops a non-system caller from seeding a read-only column lives at the protocol's create ingress (#3043). So the same forged value was dropped on `POST /data/:object` and written through on `/batch`: one rule, two answers. Create ops now go through `p.createData`, the ingress itself, rather than a second copy of the strip at the REST layer. One ingress means a future change to its policy covers the batch for free, and the carve-outs it already encodes stay intact — the platform-object exemption (a `sys_`/`managedBy` object's own guard must REJECT a forged value, not silently swallow it) and the `isSystem` exemption. `trxCtx` is passed as the context, so the insert still joins the batch transaction and `$ref` resolution is unaffected; the ingress's `droppedFields` fold into the batch's per-op list. Update ops are untouched — the engine enforces both strips on its update path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
This was referenced Aug 5, 2026
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Aug 6, 2026
) (objectstack-ai#5627) * fix(objectql): treat `autonumber` as implicitly readonly on the write path (objectstack-ai#5503) A non-system REST caller could POST an explicit record number (bypassing the sequence) and PATCH an existing one (forging a business identifier). The engine already documented that "the runtime owns the value, not the client" — and both record validators exempt a `required` autonumber because of it — but nothing on the write path enforced it. Author-declared `readonly: true` fields were already stripped (objectstack-ai#2948 / objectstack-ai#3043); `autonumber` carries no such flag, so the strip loop walked straight past it. Same defect family as objectstack-ai#4447 (`created_at`). - UPDATE: `stripReadonlyFields` now reads two sources of "read-only" at equal rank — the author-declared flag and a runtime-owned field TYPE (`isRuntimeOwnedField`, today exactly `autonumber`). Single-id and `multi` bulk updates share this one strip site, so both are covered. - INSERT: a narrower `stripRuntimeOwnedFields` runs in the engine before `applyAutonumbers`. It deliberately does NOT take over the author-declared `readonly` insert strip, which stays at the DataProtocol ingress per objectstack-ai#3413 / objectstack-ai#3043 (a create may legitimately seed read-only columns, and trusted internal writers call `engine.insert` directly). Stripping in the engine, before dispatch, is what makes this driver-agnostic: a driver advertising `supports.autonumber` receives a row with no caller value, so its persistent sequence always wins — no driver changed. The tests assert the driver-facing payload rather than patching a driver. Exemptions keep their existing semantics: `isSystem` writes skip the pass, a `preserveAudit` historical import may still reinstate legacy record numbers (objectstack-ai#3493 whitelist — `autonumber` is an author-declared business field), and only CALLER-supplied keys are candidates, so hook stamps survive. The strip is silent, so it is reported: the insert path's `onFieldsDropped` (objectstack-ai#3407) is wired at the new strip site as its standing note required, reusing the existing `readonly` reason code rather than forking the spec vocabulary. `createManyData` and `insertManyData` forward the listener too; the latter keeps row precision by attributing each dropped name to the rows that supplied it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx * test(objectql): drop two `as any` erasures on ordinary engine reads (objectstack-ai#4918 ratchet) `check:query-options-erasure` reported the test surface growing 267 -> 269: the objectstack-ai#5503 test file passed `{ where: { id } } as any` to `findOne` and `{} as any` to `find`. Neither is deliberately off-contract — both values are plain `EngineQueryOptions` that the signatures already infer — so the gate's FIRST remedy applies (drop the assertion), not the `as unknown as EngineQueryOptions` spelling reserved for tests whose subject IS off-contract input. The baseline is untouched: the count returns to its 267 ceiling on its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx * docs(changeset): record the strictReadonlyWrites superposition (objectstack-ai#5503 x objectstack-ai#5126) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3407(从 #3356 拆出的可观测性缺口)。
问题
update_record节点无条件返回success,即使请求写入的字段被数据层合法丢弃(静态readonly#2948 / 条件readonlyWhen#3042)。数据层的 warn 只落在服务端 logger,不进流程运行的步骤日志 —— 作者在 run trace 里只看到一条 3ms 的success,这正是 #3356 里审批流 stage 回写整链失效被掩盖的原因。方案拍板
采用 issue 中的**方向 1(引擎结构化回传)**的回调变体,未选方向 3(日志路由):日志路由需要把 per-run logger 一路透传进引擎与剥离函数,侵入更深,且节点只能拿到非结构化文本、拿不到字段名列表。
调研中确认的两个范围收窄:
plugin-security的detectForbiddenWrites现在直接抛PermissionDeniedError(fail-closed),会表现为success: false,不属于本单的静默剥离;create_record今天不会产生 warning —— 但按 issue 要求对称接线,未来 insert 若新增静默剥离会自动浮现,不会重新变哑。实现
spec(契约,
@objectstack/spec)data/data-engine.zod.ts:新增DroppedFieldsEventSchema——{ object, fields, reason: 'readonly' | 'readonly_when' },Zod-first、可发布进 json-schema manifest(已随构建更新);contracts/data-engine.ts:新增WriteObservabilityOptions(onFieldsDropped监听器),以交叉类型挂在IDataEngine.insert/update的 options 形参上。刻意不进可序列化的 Zod options schema:函数在 JSON Schema 不可表示(会触发 manifest ratchet 失败),也不可能跨 RPC(Virtual Data Engine)边界 —— 这是进程内通道,与IDataEngine本身同层(TS 契约层);packages/core的镜像契约同步(该文件当前无引用方,仅保持一致)。objectql(
engine.update())onFieldsDropped。对比是精确的:剥离函数在无丢弃时返回同一引用,有丢弃时返回浅拷贝;insert()出于签名对称接受该 option,但今天不触发(见上)。service-automation
NodeExecutionResult/StepLogEntry新增 advisory 的warnings?: string[],executeNode在成功/失败步骤条目上透传,随 run history 持久化(compactStepLogForHistory展开透传,无需改动);update_record/create_record:每个剥离事件生成一条 warning(点名字段与原因,readonlyWhen措辞覆盖 bulk 的「≥1 matched row」语义),并在 output 暴露结构化的droppedFields(下游节点可读{<nodeId>.droppedFields});success语义不变 —— 剥离依旧是合法语义,只是不再沉默。测试
packages/objectql/src/engine.test.ts新增 6 例:单 id readonly / 单 id readonlyWhen / bulk readonlyWhen / system 豁免不报 / 无丢弃不报 / 监听器抛错不破坏写入;packages/services/service-automation/src/builtin/crud-dropped-fields.test.ts新增 5 例:步骤 warning(成功状态保持)、droppedFields输出变量、无剥离无 warning、create_record 对称接线;json-schema.manifest.json与api-surface.json已随构建脚本再生(纯增量)。含 changeset(spec / objectql / service-automation 各 minor)与
content/docs/kernel/contracts/data-engine.mdx的契约文档更新。🤖 Generated with Claude Code
https://claude.ai/code/session_01HaofCZbsPTHE2oJedvKd77
Generated by Claude Code