Skip to content

fix(lint): report a non-system create_record / ctx.api insert of a static-readonly field — the create-verb scan gap on the flow and hook readonly rules - #16248

Merged
baozhoutao merged 3 commits into
mainfrom
claude/issue-15394-readonly-create-scan-gap
Sep 6, 2026
Merged

fix(lint): report a non-system create_record / ctx.api insert of a static-readonly field — the create-verb scan gap on the flow and hook readonly rules#16248
baozhoutao merged 3 commits into
mainfrom
claude/issue-15394-readonly-create-scan-gap

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #15394

Summary

flow-update-readonly-field and hook-api-update-readonly-field now report the static readonly shape on the create verb — a flow create_record node, and an L2 hook body's ctx.api.object('LIT').insert() — at the same error severity the static shape carries on update, under the same runAs: 'system' exemption. Both rules had scanned only the update verb, justified by the premise that INSERT is engine-exempt from the author-declared readonly strip; the maintainer's 2026-09-03 option-C ruling on #14147 (landed in #15395) made that false — engine.insert runs the same isSystem-gated stripReadonlyFields (packages/objectql/src/engine.ts insert path, staticReadonlyInsertSubject) — so a non-system create of a readonly column became a silent no-op that nothing reported at build time.

Clause-②: yes — two published os lint rules start reporting a new error-severity finding, narrowing what os lint / os validate / os build accept. Label needs:contract-review applied.

What now fails that passed before (measured blast radius)

Exactly one new shape per rule, at error:

  • a create_record node whose literal fields map writes a field the target object declares readonly: true, on a flow without runAs: 'system';
  • an L2 hook body's literal ctx.api.object('LIT').insert({ … }) writing such a field, on a hook without runAs: 'system'.

Blast radius over this repository at origin/main 159dbad (git grep create_record over examples/** and packages/**, non-test, crossed with every readonly: true field declaration under examples/**; hook bodies grepped for ctx.api.object(...).insert|create():

site target object readonly fields on it writes any? runAs verdict
examples/app-showcase/src/automation/flows/index.ts:1407 showcase_inbound_task_webhook › create_task showcase_task none system clean
examples/app-todo/src/flows/task.flow.ts:236 task_completion › create_next_task todo_task completed_date no (user) clean
examples/app-todo/src/flows/task.flow.ts:317 quick_add_task › create_task todo_task completed_date no (user) clean
hook bodies with ctx.api…insert() / .create() in examples/** or packages/** (non-test) 0 sites (only CHANGELOG / comment mentions) clean

app-crm (crm_opportunity.approval_status readonly) declares no create_record; app-showcase contact.lead_score is readonly but nothing creates a contact from a flow or hook. Zero in-repo sites go red; no example file was touched. Out-of-repo consumers (e.g. hotcrm) learn from the changeset, which carries the migration (declare runAs: 'system' when seeding the column is the intent, otherwise drop the key or stamp it in a beforeInsert hook).

Rulings executed and mechanism assumptions verified

  • Static shape only on create. engine.ts still reads "INSERT stays exempt" for the readonlyWhen strip (the insert-path note and the bulk stripReadonlyWhenFields site), so no conditional finding is produced on a create, on either surface; pinned green with an update-verb control in both test files.
  • Action rule untouched. READONLY_ACTION_INSERT_SILENCE exists on origin/main (validate-readonly-action-writes.ts:182, pinned at its test :304).
  • A1 (flow object anchor): CreateRecordConfigSchema in packages/spec/src/automation/builtin-node-config.zod.ts anchors on objectName with fields: record, the same keys as update; the flow-node-crud-object-alias conversion covers the whole CRUD quartet (packages/spec/src/conversions/registry.ts:920), so the existing readLiteralObjectName (canonical + alias) is the right read. Verified — no new anchor code.
  • A2 (hook create verb) — half falsified. The spec ObjectRepository contract (packages/spec/src/contracts/scoped-context.ts:154) declares insert only; the host ObjectRepository (packages/objectql/src/engine.ts) aliases create() to insert(); but this rule reads L2 bodies, which run in QuickJS, and installCtx (packages/runtime/src/sandbox/quickjs-runner.ts:556) installs insert / update / delete / updateMany / deleteMany / upsertno create leaf. A body's .create() is therefore TypeError: not a function on its first run: loud, not silent. So the hook rule widens to insert only; create is recorded as a reasoned method exclusion (READONLY_HOOK_METHOD_EXCLUSIONS) and pinned, and the partition against the extractor's declared verb list is pinned too. The card's insert / .create shorthand was hedged for a reason.
  • A3: message text follows create-record-readonly-drop.test.ts: the row is created without the column, the step reports success, a run-time warning names the field.
  • A4: consumers typechecked with the prefix filter pnpm --filter '...@objectstack/lint' typecheck — the downstream direction (see Verification).
  • A5: git log origin/main --since=2026-09-04 -- packages/lint/src/validate-readonly-*b398ad25 (fix(objectql,metadata-protocol): a static readonly field is stripped from a non-system INSERT inside engine.insert, and the boundary copy is deleted #15395, rewrote both headers) and 7dafaaed (fix(lint): no authoring rule throws on a non-record entry of any stack collection #15751); worktree cut from 159dbad, after both.

Design notes

  • One rule id per shape, not per verb. The finding is the same fact (a caller-supplied write to a declared-readonly field that the same strip removes), so the existing ids carry the create verb and the message names the verb it was judged on. A second id would only split one finding's suppression, docs and counts.
  • Under runAs: 'system' a create_record writing a field declaring BOTH readonly and readonlyWhen is clean (no static strip by elevation, no conditional strip on INSERT), where the same update_record still draws the conditional warning — pinned, with the update control.
  • id in an insert payload is not the row address (The read-only strip still logs a WARN calling the addressed row's own id a forged caller write, on every single-record update of a platform object #8141 applies to the payload-addressed update only) — pinned.

Surface

Declared: packages/lint/src/validate-readonly-flow-writes.ts, validate-readonly-hook-writes.ts, their tests, validate-flow-node-writes.test.ts (comment of the GREEN control only), one changeset (@objectstack/lint: minor, following the precedent of decision-predicate-envelope-refused / expression-source-non-string-refused for a new build-time refusal).

Declared increment: content/docs/automation/hook-bodies.mdx (two sentences at the rule bullet and the scope paragraph). The paragraph literally said "nothing reports it at build time yet (#15394)"; merging a PR that closes #15394 while that sentence stands would leave the published docs asserting a gap that no longer exists, so the two sentences are updated in the same landing. No other doc names the create-side gap (content/docs/data-modeling/fields.mdx:319 already states the engine behaviour correctly).

维护者速读(草稿)

改了什么。 os lint 的两条 readonly 规则(flow 的 flow-update-readonly-field、hook 的 hook-api-update-readonly-field)现在也检查"创建"动作:非 runAs: 'system' 的 flow create_record 节点、或 hook body 里的 ctx.api.object('…').insert(),若写入对象声明为 readonly: true 的字段,按 error 报出,与 update 同级。仅报静态 readonly;readonlyWhen 在创建时不报(引擎在 INSERT 上不跑条件剥离)。hook 侧只覆盖 .insert(),不覆盖 .create():沙箱里没有 create 这个方法,调用会直接抛错,不是静默丢失。

为什么改。 2026-09-03 的 option C 裁决(#14147)让 engine.insert 也跑 readonly 剥离后,非系统身份创建记录时写入的 readonly 字段会被静默丢弃、步骤仍报成功;两条规则的注释已改口称之为"扫描缺口",但规则本身没补。本 PR 把缺口补上,让作者在构建期而不是运行日志里发现问题。

风险与代价(含回滚)。 收紧了 os lint / os build 的接受面:以前通过的一类写法现在报 error。仓内三个示例应用实测零命中(表见上文),外部消费方(如 hotcrm)可能有需要处理的站点——修法是声明 runAs: 'system'(确实要由自动化写入时)、删掉该字段、或改用目标对象自己的 beforeInsert 钩子打戳。回滚即 revert 本 PR,规则退回只查 update。

席位意见。 (留空,待席位定稿)

你要做的。 确认"创建动作的静态 readonly 写入按 error 报"这一收紧方向,以及 hook 侧只覆盖 .insert() 不覆盖 .create() 的取舍;确认后由队列合并(非受管面)。

Verification

All at final head 9b76e9a49 (branch claude/issue-15394-readonly-create-scan-gap, base origin/main 159dbad), in the issue worktree; every build/test through scripts/pm/os-verify-lock.sh, exit codes captured before any pipe.

  • pnpm --filter '@objectstack/lint^...' buildVERDICT command-exit 0.
  • pnpm --filter @objectstack/lint buildDTS Build success, VERDICT command-exit 0.
  • Targeted: pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/validate-readonly-flow-writes.test.ts src/validate-readonly-hook-writes.test.ts src/validate-flow-node-writes.test.tsTest Files 3 passed (3), Tests 106 passed.
  • Full: pnpm --filter @objectstack/lint testTest Files 98 passed (98), Tests 3375 passed (3375).
  • pnpm --filter @objectstack/lint typecheck — exit 0 (tsc --noEmit + check:test-typecheck: OK).
  • Downstream consumers (prefix filter = every package that depends on lint, built via turbo ^build): pnpm exec turbo run typecheck --filter='...@objectstack/lint' --concurrency=2Tasks: 118 successful, 118 total, VERDICT command-exit 0.
  • packages/cli unit tier (direct consumer): pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2Test Files 179 passed (179), Tests 2406 passed | 6 expected fail (2412). The integration tier is declared to CI (no diff on it).
  • Ablation (commit first, mutate, prove on disk, restore from HEAD): both subject sets reverted to update-only (READONLY_FLOW_WRITE_NODE_TYPES[UPDATE_NODE_TYPE], READONLY_HOOK_STRIP_SUBJECT_METHODS['update','updateById']); on-disk blob hashes changed (213191a1236f5f31, c3ec36f0f8a90e0b) with the injected marker counted 1 and the removed anchor 0 per file; the two flipped test files went red on exactly the 11 create/insert cases (Tests 11 failed | 63 passed (74)); restore by git checkout HEAD -- PATH, proven by git diff HEAD empty, git status --porcelain empty and on-disk hashes equal to the HEAD blobs. The tests import the rules relatively from src, so no dist is on the ablation path.
  • Gate sweep: node scripts/pm/dispatch-gates.mjs --changed --commands derived 81 families at 33ca99a5 (list byte-identical when re-derived at 9b76e9a49); all 81 run, --ran reconciles 81 derived famil(ies) accounted for — 81 run, 0 NOT-MEASURED. One real red found and fixed in-flight: check:doc-authoring (tracker id #14147 inside the new runtime message string — moved to the adjacent comment; green on rerun, 829 pinned sites, no growth). Reran green on the final tree: check:nul-bytes, check:cross-package-test-inputs, check:test-source-alias, check:doc-authoring, check:docs-transcript-drift, check:dts-closure (66 packages swept, 157/157), check-undeclared-dep-imports, check-affected-docs, check-drift-comment, lint's check:doc-formula-expressions / check:doc-security-posture, spec's check:skill-examples (after building client-react), check-plugin-teardown-shape --self-test (after deepening the clone to its pinned commit). check-adr-0087-registration --base origin/main: "adds no declared-breaking changeset (1 non-breaking changeset(s) seen)" — no disposition marker owed. check-changeset-no-major / check-empty-changeset: green.
  • NOT MEASURED locally, CI-owned: check:dual-build-cjs-loads and check:type-check-debt exit 3 PREREQUISITE NOT MET — both read every package's built output and this container did not build the whole repo (84 packages without dist); the diff touches none of the packages they ledger.
  • Narrowed eslint (repo scan is CI's): eslint.config.mjs:328 states no parserOptions.project / typed rules, so the diff cannot move any untouched file's verdict; pnpm exec eslint --no-inline-config --format json over the 5 changed .ts files at 9b76e9a49 — 5 files, 0 errors, 0 warnings; .changeset/*.md and content/docs/**/*.mdx are outside eslint's population ("File ignored because no matching configuration").
  • Docs gates on the increment (content/docs/automation/hook-bodies.mdx): check:docs-transcript-drift (4 declared values equal the registry), check-doc-frontmatter, check-doc-route-spelling, check-docs-section-name, check:doc-anchors, check:docs-single-h1 all green.
  • Same-day churn (A5): git log origin/main --since=2026-09-04 -- packages/lint/src/validate-readonly-*b398ad25 (fix(objectql,metadata-protocol): a static readonly field is stripped from a non-system INSERT inside engine.insert, and the boundary copy is deleted #15395), 7dafaaed (fix(lint): no authoring rule throws on a non-record entry of any stack collection #15751); base 159dbad postdates both.
  • mcp_calls: 0 — every GitHub read and write went through repo-scoped REST.

🤖 Generated with Claude Code


Generated by Claude Code

…he flow and hook readonly rules

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
…hich the engine's insert strip does not judge

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 618f70d74d934c09fa58f1eaa8493e95e4f847e9.

2 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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 618f70d74d934c09fa58f1eaa8493e95e4f847e9packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 618f70d74d934c09fa58f1eaa8493e95e4f847e9

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 6, 2026
@baozhoutao
baozhoutao marked this pull request as ready for review September 6, 2026 08:14
@baozhoutao
baozhoutao enabled auto-merge September 6, 2026 08:14
@baozhoutao
baozhoutao added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 720bf47 Sep 6, 2026
43 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-15394-readonly-create-scan-gap branch September 6, 2026 08:49
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

2 participants