Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

手写 FieldValidationRules 在唯一读点既不校验也不归一:pattern.value 写成 string(类型明确允许)被 react-hook-form 静默忽略,未识别的规则名同样静默丢弃 #5099

Closed

Description

@yinlianghui

发现于 #5075 的实施(那一单是 README 键面,本卡是类型与运行时的契约缺口,不同性质,故未夹带进 PR)。Filed unassigned, not claiming.

现象

FieldValidationRules.pattern(packages/types/src/form.ts:765 附近)声明为:

pattern?: { value: string | RegExp; message: string };

string 是声明允许的。但手写 schema 这一路没有任何地方把它编译成正则:基础 form 渲染器的唯一读点是原样展开 —— const rules: any = { ...validation }(packages/components/src/renderers/form/form.tsx:1652),此后只有 localizeRule 填 message(:1752),value 不动。而 react-hook-form 7.85.0 的字段校验器对 pattern 只在 value instanceof RegExp 时才应用:

Z=e=>e instanceof RegExp
… if(V&&!L&&I(E)){const{value:e,message:t}=oe(V); if(Z(e)&&!E.match(e)&&…

(node_modules/.pnpm/react-hook-form@7.85.0_.../dist/index.cjs.js,Z 即 isRegex)

于是 validation: { pattern: { value: '^[a-zA-Z0-9_]+$', message: '…' } } —— 一个类型完全合法的声明 —— 一条也不跑,零报错。

对象元数据那一路是好的,因为它归一化:buildValidationRules(packages/fields/src/index.tsx:2246,:2303)写的是 value: typeof field.pattern === 'string' ? new RegExp(field.pattern) : field.pattern。所以「string 能用」只在元数据路线上为真,而类型把这个保证发给了两条路。

同一读点的第二个静默面(机制相同)

{ ...validation } 不做任何键名校验,react-hook-form 的字段校验器只解构固定集合:

const{ref:c,refs:f,required:m,maxLength:y,minLength:p,min:g,max:b,pattern:V,validate:A,name:x,valueAsNumber:S,mount:k}=e._f

任何未识别的规则名(拼错的 minlength、发明的 email、以及 #5075 第 1 组那种数组展开出来的数字键)都被静默丢弃。表单看上去有校验,实际一条不跑。这正是 AI 生成的元数据最容易藏错的地方。

两个方向,请维护者定

A. 收紧声明(producer 侧严格):pattern.value 在手写面收成 RegExp,string 只留给元数据路线的字段声明(FieldSchema.pattern,由 buildValidationRules 编译)。

  • 长期健壮性:与 AGENTS.md #0.1 一致 —— 声明即强制,错误在授权时就被拒,不靠消费端容忍。
  • 让 AI 写的元数据难写错:最强 —— 写 string 直接编译红,不会留下一个「看起来有校验」的表单。
  • 代价:FieldValidationRules 是公开类型,收紧是 breaking(按仓库版本策略仍标 minor,正文写清 breaking 语义);已有手写 string 的调用方会转红 —— 但它们今天本来就没在校验,转红是把静默失效换成显式失败。

B. 在唯一读点归一 + 对未识别键报错:{ ...validation } 处把 string pattern 编译成 RegExp,并对不在 react-hook-form 识别集合内的规则名 console.error(或开发期 throw)。

  • 长期健壮性:较弱 —— 这是消费端容忍,等于把 string | RegExp 这个二义声明固化成契约;但「对未识别键报错」这半是纯增益,任何选择下都值得做。
  • 让 AI 写的元数据难写错:中等 —— 错误在运行时才发现,但至少不再静默。

倾向 A + B 的报错半:A 关掉二义声明(声明即强制),B 的未识别键报错留下一道运行期兜底,让 #5075 那类数组拼法在开发期就喊出来而不是静默。⛔ 不建议只做 B 的归一化那半 —— 那是给一个本该收紧的声明加宽容,正是 #0.1 点名的模式。

复核方式

sed -n '760,775p' packages/types/src/form.ts                      # pattern.value: string | RegExp
sed -n '1650,1655p;1748,1755p' packages/components/src/renderers/form/form.tsx   # 唯一读点,value 不动
sed -n '2300,2312p' packages/fields/src/index.tsx                 # 元数据路线 new RegExp(...)
grep -oE "Z=e=>e instanceof RegExp" node_modules/.pnpm/react-hook-form@*/node_modules/react-hook-form/dist/index.cjs.js

分级

交 PM 分诊,未打 finding 标签:不是休眠代码 —— 任何按公开类型手写 pattern: { value: '…' } 的调用方今天就拿到一个静默不校验的字段。与 #5075 第 1 组同族(同一读点、同一「零报错」症状),但那一单只改文档、不动渲染器与类型,所以这一半单独立卡。

Activity

  1. yinlianghui commented on Aug 17, 2026

    @yinlianghui
    ContributorAuthor

    立卡人补记(issues 端点会剥掉正文末尾 footer,改用评论补):本卡由 #5075 的 os-dev 席在实施中扫出并如实分离 —— 那一单纯文档,不动 packages/types 与 packages/components;README 侧的处置是教能跑的那种拼法(hand-authored pattern.value 写 RegExp),把类型/运行时的契约选择留给本卡。未认领。


    Generated by Claude Code

  2. yinlianghui commented on Aug 18, 2026

    @yinlianghui
    ContributorAuthor

    PM 分诊(session_01GTRjn8xBqp75dk7kFupVRt,objectui 分片):挂 bug + needs-user-decision,入决策箱。手写 FieldValidationRules 在唯一读点既不校验也不归一(pattern.value 为 string 时被 react-hook-form 静默忽略、未识别规则名静默丢弃),而类型声明明确允许 string —— 收敛方向是公开契约题:

    在裁定落地前,README 侧(PR #5100 已落)按已实测事实教「hand-authored schema 传 RegExp」,不预断本卡方向。


    Generated by Claude Code

  3. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    Contributor

    Maintainer ruling recorded — maintainer, 2026-08-18, batch acceptance of the triage seat's decision-inbox recommendations, verbatim: 「同意」. Provenance: triage session session_pm_triage_20260818T0104Z (objectstack#6015).

    Ruled: A + B's error half; ⛔ not B's normalization half. Concretely: (1) FieldValidationRules.pattern.value narrows to RegExp on the hand-written surface — string stays only on the metadata route's field declaration, which buildValidationRules compiles; a hand-written string pattern becomes a compile-time error instead of a silently-unvalidated field. (2) The single read point additionally reports unrecognized rule names (dev-time error) so misspellings and invented rules shout instead of vanishing. (3) The normalization half of B is explicitly rejected — compiling strings at the read point would harden the ambiguous declaration into contract, the consumer-tolerance shape #0.1 names. Changeset per repo policy with the breaking semantics stated plainly in the body: callers writing string patterns turn red, which converts silent non-validation into explicit failure.

    Label needs-user-decision → pm:queue in the same stroke.


    Generated by Claude Code

  4. self-assigned this
    on Aug 18, 2026
  5. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator

    Claim: PM loop round 1 (batch raised 3 → 4 on the maintainer's 2026-08-18 standing directive to prioritise bugs and UI/UX work — recorded on the seat post)
    Session: session_01AMHbqfPiETJHA95r6WzZWS
    Branch: claude/issue-5099-validation-rules-contract
    Worktree: objectui-issue-5099
    Domain: repo:objectui (execution seat; this repo has no domain:* lanes)
    File surface: packages/types/src/form.ts (the FieldValidationRules declaration), packages/components/src/renderers/form/form.tsx (the single read point, ~:1652 / ~:1752), plus tests. packages/fields/src/index.tsx's buildValidationRules is read-only reference — the metadata route is correct and is not being changed (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: claude-fable-5
    Clause-②: yes

    Why Clause-②: yes and why fable — stating it because the path check alone would have missed it. dispatch-gates.mjs --tier reports no path-derived mandate for these paths, and this repo has no packages/spec/**, so the path limb is silent. But the content limb is not: the ruled fix changes the accept/reject behaviour of a published contract surface — FieldValidationRules.pattern.value narrows from string | RegExp to RegExp, so a hand-written declaration that compiles today becomes a compile error. That is clause ②'s content limb regardless of which directory it lives in, and clause ② is written as "changes accept/reject behaviour or widens the public surface" — narrowing counts. Dispatching at the contract-review tier up front, which is the cheaper end of being wrong here.

    Serial constraints cleared: no open PR and no in-flight claim touches packages/types or packages/components. Disjoint from this round's other three claims (#5176 app-shell, #5141 fields, #5163 plugin-map). Note packages/components last moved in PR #4933 (dad51e5dc, merged) — landed, not in flight.

    The ruling is already made — implement it, do not re-open it

    Maintainer ruling of 2026-08-18 (comment-5322881617, verbatim 「同意」) is binding and complete:

    1. A — FieldValidationRules.pattern.value narrows to RegExp on the hand-written surface. string survives only on the metadata route's field declaration (FieldSchema.pattern), which buildValidationRules compiles.
    2. B's error half — the single read point additionally reports unrecognized rule names at dev time, so misspellings (minlength) and invented rules (email) shout instead of vanishing.
    3. ⛔ B's normalization half is explicitly REJECTED. Do not compile string patterns at the read point. The ruling's reason, which is the point of the card: doing so would harden an ambiguous declaration into contract — the consumer-tolerance shape AGENTS.md #0.1 names. If you find yourself adding new RegExp(...) in form.tsx, you have implemented the rejected option.

    Premise check — run now against origin/main @ 072085de, all three sites confirmed live

    packages/types/src/form.ts:744            export interface FieldValidationRules {
    packages/types/src/form.ts:942              validation?: FieldValidationRules;
    packages/components/src/renderers/form/form.tsx   ← the single read point ({ ...validation })
    packages/fields/src/index.tsx             ← buildValidationRules, the CORRECT metadata route
    

    Line numbers inside those files are from the card and are not re-verified — take them from the tree.

    Non-negotiables

    • The no-major gate is real (scripts/check-changeset-no-major.mjs). The ruling says: changeset per repo policy, with the breaking semantics stated plainly in the body. Narrowing a published type while the repo forbids a major bump is the delicate part of this card — solve it deliberately and explain the choice in the PR, do not paper over it.
    • Turning existing callers red is the intended outcome, not collateral: those callers are not validating anything today. If the narrowing turns in-repo callers red, fix them to pass RegExp — that is in surface. If it turns up a caller where a RegExp genuinely cannot be supplied, stop and report rather than widening the type back.
    • The console.error for unrecognized rule names must key off the set react-hook-form actually destructures (the card quotes it from the pinned 7.85.0 bundle). Derive that set from the installed version, and pin it so a react-hook-form bump that changes the set does not silently rot the diagnostic.
    • Do not touch the metadata route. It is correct.

    Deliver

    Draft PR against main, first line Fixes #5099. Push the branch early.

    Return the structured JSON report AND post it as an issue comment whose first line is <!-- os-dev-report -->. Machine-checkable fields only — gates run + outcomes, files_changed, deviations, tests with real output.


    Generated by Claude Code

  6. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator
    {
      "issue": 5099,
      "status": "done",
      "branch": "claude/issue-5099-validation-rules-contract",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5187",
      "premise_still_valid": true,
      "summary": "All three ruled limbs implemented at 0b27ef102: (A) FieldValidationRules.pattern.value narrowed to RegExp in packages/types/src/form.ts; (B-error) the form renderer reports unrecognized validation rule names via console.error, recognized set in validationRuleKeys.ts pinned against the installed react-hook-form 7.85.0 bundle by a test that re-derives the ._f destructure at test time; (B-normalization) NOT implemented per the ruling, and pinned by a test asserting string patterns reach react-hook-form uncompiled. Zero in-repo callers turned red. Card body checked for truncation: complete except the author-acknowledged stripped footer. Breaking-vs-no-major reconciled per AGENTS.md version policy: minor bump, breaking semantics stated in changeset body and PR.",
      "files_changed": [
        "packages/types/src/form.ts",
        "packages/components/src/renderers/form/form.tsx",
        "packages/components/src/renderers/form/validationRuleKeys.ts",
        "packages/types/src/__tests__/field-validation-rules-pattern-regexp.test.ts",
        "packages/components/src/renderers/form/__tests__/rhf-recognized-rule-keys.test.ts",
        "packages/components/src/renderers/form/__tests__/unrecognized-validation-rules.test.tsx",
        ".changeset/field-validation-rules-pattern-regexp-5099.md"
      ],
      "gates": [
        "vitest new tests (3 files): 9 passed (9) — PASS",
        "vitest packages/components/src/renderers/form/ + packages/types/ @ 0b27ef102: Test Files 80 passed (80), Tests 683 passed (683) — PASS",
        "pnpm --filter @object-ui/types type-check (tsc --noEmit + examples + test tsconfigs): PASS",
        "pnpm --filter @object-ui/components type-check: PASS",
        "turbo run type-check (all packages, consumer radius): 81 successful, 81 total — PASS",
        "turbo run lint --filter types --filter components: 3 successful, 3 total — PASS",
        "check-changeset-presence.mjs: 6 src files of 2 released packages, 1 changeset declared — PASS",
        "check-changeset-fixed.mjs + check-changeset-no-major.mjs: PASS",
        "check:control-bytes: scanned 4596 tracked text files — PASS",
        "check:doc-snippets (after full workspace build): 67 of 67 blocks judged, 0 failed — PASS",
        "check:doc-types: PASS; check:phantom-deps: PASS; check:self-import: PASS",
        "remote CI on PR #5187: in_progress at report time (report delivered at draft-PR time per contract)"
      ],
      "tests": "New tests first run: 'Test Files 3 passed (3), Tests 9 passed (9)'. Union at final commit 0b27ef102: 'Test Files 80 passed (80), Tests 683 passed (683)'. Reverse verifications from committed state: (1) cross-package probe — temp components file with string pattern, tsc red: \"error TS2322: Type 'string' is not assignable to type 'RegExp'\", probe deleted; (2) type ablation — form.ts restored from origin/main, types tsc red with 3 errors incl. \"TS2578: Unused '@ts-expect-error' directive\", restored; reader is tsc over src, no dist in path, no rebuild needed; (3) diagnostic ablation — form.tsx restored from origin/main, observed '2 failed | 5 passed': the two reporting tests red, the three behavior-identical-on-main tests green (expected direction, not all-red), bundle-pin test green; reader is vitest over relative src imports, no dist in path; restored. The one dist-reading pair (components tsc reading types .d.ts) was exercised by leg 1 after a real dependency-closure build.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5186: FormFieldSchema.validation's zod mirror (FieldConstraintsSchema) contradicts FieldValidationRules in both directions — rejects the TS-legal {value,message} object shape, accepts a flat shape the renderer never reads (measured against dist)"
      ],
      "deviations": []
    }

    Generated by Claude Code


    Generated by Claude Code

  7. added a commit that references this issue on Aug 18, 2026
  8. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator
    {
      "issue": 5099,
      "status": "done",
      "branch": "claude/issue-5099-validation-rules-contract",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5187",
      "premise_still_valid": true,
      "summary": "PM patch round applied at d7b741f4f (on top of accepted 0b27ef102), both blocking changes: (1) the unrecognized-rule-name diagnostic is now dev-only — process.env.NODE_ENV === 'production' returns first, matching the warnDeprecatedOnce shape in basic/div.tsx and basic/span.tsx including its documented order property (production check returns BEFORE the seen-set is marked, so a production render never suppresses the later dev notice); (2) it reports once per field + rule-set via a module-level Set memo keyed fieldName + sorted offending keys — the warnOnTopLevelStyleUrl shape from plugin-map/ObjectMap.tsx on current main. Message text, RHF_RECOGNIZED_RULE_KEYS extraction, bundle pin test, type narrowing, changeset, and the metadata route all unchanged. Both new behaviors pinned by tests, gate order included. PR left as draft.",
      "files_changed": [
        "packages/components/src/renderers/form/form.tsx (patch round: dev gate + dedupe memo, effect body moved into module-level reportUnrecognizedRulesOnce)",
        "packages/components/src/renderers/form/__tests__/unrecognized-validation-rules.test.tsx (patch round: +2 pins — production gate incl. order property, dedupe incl. keys-in-memo)"
      ],
      "gates": [
        "vitest packages/components/src/renderers/form/ + packages/types/ @ d7b741f4f: Test Files 80 passed (80), Tests 685 passed (685) — PASS",
        "targeted 3 files (diagnostic behavior + bundle pin + type pin): 11 passed (11) — PASS",
        "pnpm --filter @object-ui/types type-check: PASS; pnpm --filter @object-ui/components type-check: PASS",
        "turbo run type-check (all packages, consumer radius): 81 successful, 81 total — PASS",
        "turbo run lint --filter types --filter components: 3 successful, 3 total — PASS",
        "check-changeset-presence.mjs: 6 src files of 2 released packages, 1 changeset — PASS; check-changeset-fixed.mjs: PASS; check-changeset-no-major.mjs: PASS",
        "check:control-bytes: scanned 4596 tracked text files — PASS",
        "remote CI on PR #5187: in_progress at report time (draft retained for PM re-review per patch-round instruction)"
      ],
      "tests": "Updated suite at d7b741f4f: 'Test Files 80 passed (80), Tests 685 passed (685)' (was 683; +2 new pins). New-pin ablation from committed state: form.tsx restored to HEAD~1 (the accepted-but-ungated version) → exactly the two new tests red — '× is silent in production builds, and the gate does not eat the dev notice', '× reports once per field + rule-set, however often the caller rebuilds fields', '2 failed | 5 passed' — all pre-existing tests green, then restored from branch HEAD; reader is vitest over relative src imports, no dist in path, no rebuild needed. Gate-order property is itself pinned: production render first (0 messages), unstub env, rerender with fresh fields array → 1 message, which goes red if the seen-set is marked before the production return.",
      "open_questions": [],
      "out_of_scope_findings": [
        "previously filed as #5186: FormFieldSchema.validation's zod mirror (FieldConstraintsSchema) contradicts FieldValidationRules in both directions (unchanged this round)"
      ],
      "deviations": [
        "check:doc-snippets not re-run this round: the patch touches only components renderer source and a test; doc snippets compile against built types, which are byte-identical to the run that passed 67/67 at 0b27ef102"
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  9. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator

    ACCEPT after one patch round — PR #5187 (repo:objectui seat PM, session session_01AMHbqfPiETJHA95r6WzZWS, reviewer of record).

    The patch round, and the thing it found that I did not ask for

    I sent this back for two blocking changes: gate the new diagnostic to non-production (the ruling said dev-time, the implementation fired unconditionally), and stop it re-firing per render. Both applied at d7b741f4f, no argument — and I checked, the argument would not have won.

    What matters is what it did beyond the instruction. I pointed it at basic/div.tsx / basic/span.tsx as the in-package idiom. It matched not just the shape but the documented ordering property those files carry:

    if (process.env.NODE_ENV === 'production') return;   // BEFORE the memo is marked
    if (_reportedUnrecognizedRules.has(memo)) return;
    _reportedUnrecognizedRules.add(memo);
    

    div.tsx's own comment explains why that order is load-bearing — "the production check returns BEFORE the seen-set is marked, so a production render never suppresses the dev notice" — and records the incident behind it (a browser-verification run that lost the signal it was looking for). Mark-then-return would poison the memo in a production render and silently cost the dev the warning. I did not specify this; it read the precedent's reasoning rather than copying its syntax, and pinned the ordering with a test (production render → 0 messages → unstub env → rerender → 1 message, red if the order flips). That is the difference between following an instruction and understanding it.

    The ruled limbs, verified in the diff rather than from the report

    • A — FieldValidationRules.pattern.value is { value: RegExp; message: string }. Confirmed, and FieldSchema.pattern: string is untouched — the metadata route the ruling explicitly protected still takes a string and still compiles it via buildValidationRules.
    • B, error half — unrecognized rule names reported against RHF_RECOGNIZED_RULE_KEYS, extracted to validationRuleKeys.ts and pinned against the installed react-hook-form bundle by a test that re-derives the ._f destructure at test time, so a future bump cannot silently rot the diagnostic.
    • B, normalization half — correctly ABSENT. I grepped: there is no new RegExp anywhere in form.tsx. The rejected option stayed rejected, and is itself pinned by a test asserting string patterns reach react-hook-form uncompiled. Pinning the absence of a rejected option is unusual and right — it is what stops a later well-meaning refactor from "helpfully" adding tolerance back.

    The message text is the other quiet strength: it doubles as the fix instruction, naming the correct spelling, the RegExp requirement, and the array-spread trap. That matters precisely because limb A only protects TypeScript authors — and the metadata that reaches this renderer as plain JSON never met a compiler. The type guards humans; the diagnostic guards generated metadata.

    Semver — the genuinely hard part, solved by reading policy rather than dodging it

    Narrowing a published type while check-changeset-no-major.mjs forbids a major is a real conflict. It shipped minor with the breaking semantics spelled out in the changeset body, and I verified the justification independently rather than accepting it: AGENTS.md §203–207 does say objectui's major is pinned to @objectstack's across a 39-package fixed group, and that "objectui 自身的破坏性变更也标 minor(在正文里写清 breaking 语义即可)". The changeset does exactly that, and states what breaks, why red is the fix rather than the damage, and what is unaffected. Correct by policy, not by workaround.

    Zero in-repo callers turned red, which is itself the finding that the old type was decorative: every caller was already passing a RegExp or not validating at all.

    Evidence

    Three reverse verifications, each with the right reader named — a cross-package probe proving consumers read the rebuilt .d.ts after a real dependency-closure build, a type ablation, and a diagnostic ablation. Two of them explicitly confirm no dist sits in the path, which is the failure mode that makes a green ablation meaningless. Plus a fresh ablation this round isolating exactly the two new pins.

    Clause ②

    Declared Clause-②: yes at claim time and dispatched at claude-fable-5 — which is CONTRACT_REVIEW_TIER. So the enqueue gate does not bind: the dispatch tier was at the floor, not below it, and no needs:contract-review hold is owed. Recording this because it is the case the gate is designed to produce — paying the tier up front on a card that changes a contract's accept/reject behaviour costs one dispatch and avoids a blocked queue entry later.

    Landing: path surface is packages/types/** + packages/components/** + changeset — no governed face, queue-eligible. CI re-running for d7b741f4f (11 terminal, 10 in progress). Flip and enqueue on terminal success of every required job, read by name.

    Out-of-scope finding #5186 stands from the first round, unchanged.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions