Uh oh!
There was an error while loading. Please reload this page.
fix(console): honour the field-level maxLength override in buildSections - #5635
Merged
os-sales merged 1 commit intoAug 21, 2026
Merged
Conversation
…ons (#5595) `buildSections` merges a FormView's field overrides with the object's field definitions, and its docstring promises field-level overrides take precedence. Every key implemented that — `override.label ?? def.label`, `override.required ?? def.required`, … — except `maxLength`, which read `def.maxLength` unconditionally, so a tighter per-form ceiling was discarded with no diagnostic. The merged row reaches the DOM at two `maxLength={field.maxLength}` sites, so the symptom is a value the author believed the input refused being accepted. `??` rather than `||`, matching the siblings: an explicitly declared `0` stays a value the author wrote. The override can only narrow what the input allows; the object's storage ceiling still decides at submit time. The #5542 pin `expect(row.maxLength).toBeUndefined()` recorded the OLD answer and is INVERTED to `toBe(40)` rather than deleted — it is the pre-registered evidence for this fix and is what made the gap findable. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 21, 2026 21:10
Uh oh!
There was an error while loading. Please reload this page.
os-sales
deleted the
claude/issue-5595-buildsections-maxlength-override
branch
August 21, 2026 21:10
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#5595
The defect
apps/console/src/components/FormPage.tsxbuildSectionsmerges a FormView's fieldoverrides with the target object's field definitions. Its own docstring states the rule:
Every key in the loop is built that way —
override.label ?? def.label,override.required ?? def.required,override.placeholder ?? def.placeholder, … — exceptone, which read the object definition unconditionally:
So an author who set a tighter per-form limit (a short public intake form over a column
whose object-level ceiling is generous) silently got the generous one. No diagnostic, no
warning, and the form still submits — the symptom is a value the author believed the input
refused being accepted.
It is load-bearing rather than decorative: the merged row reaches the DOM at two
maxLength={field.maxLength}sites, thetextareaarm and the defaultinput type="text"arm.
The fix, per triage's ruling on the fork the card posed — honour the override:
??rather than||, matching the sibling keys, so an explicitly declared0stays avalue the author wrote. This is a declared≠enforced restoration, not a feature: the
docstring already promised it and every sibling key already implemented it. The override
can only narrow what the input allows — the object's storage ceiling still decides at
submit time, so nothing accepted before is now rejected anywhere but at the keyboard.
Line numbers re-derived on current
origin/main— all four had driftedThe card cited
:340/:367/:875/:983against the merged refcad512fe1.#5594 (PR #5624) has since landed in this same file, adding predicate wiring, an exported
isFieldVisibleand aRenderableFieldfield. Located by code, not by number, atf52d36c96:cad512fe1)f52d36c96)Field-level FormField overrides take precedence…maxLength: def.maxLength,maxLength={field.maxLength}—textareaarmmaxLength={field.maxLength}— defaultinputarmAll four drifted, all in the same direction, by 53–103 lines. The mechanism the card
described was otherwise exactly right.
The pre-registered pin: INVERTED, not deleted
FormPage.fieldSpec.test.ts(added by #5542) carriedexpect(row.maxLength).toBeUndefined()— recording the OLD answer explicitly rather thanassuming it. That assertion is inverted to
expect(row.maxLength).toBe(40), and keptin place.
Inverting rather than deleting, deliberately: the assertion is what made this gap findable
at all. #5542 chose to record whether each newly-declarable key was honoured instead of
assuming it, and the one recorded as not-honoured became this card. Deleting it would
retire the mechanism that produced the finding; keeping it means the same line now names
the honoured answer. Its
objectSchemaargument isnull, so the40can only have comefrom the field override.
The file's own docstring said "the keys this renderer does not honour are recorded as not
honoured rather than assumed" — updated in the same commit, since that sentence is no
longer true of this key.
Reverse verification — three legs, mutation confirmed on disk in both directions
The fix was committed first, so every leg restores against a real commit. Each leg is a
script carrying
trap restore EXIT INT TERM, and each asserts anchoredgrep -ccountsfor both the fixed line and the mutant line before it measures anything — an editor's
exit code is not evidence a replacement landed.
No rebuild leg is required here, and that is a property of the imports rather than an
omission: the tests import
./FormPage, a relative path inside the same app, not anexports-resolved package boundary. Nothing resolves through anydist/, so the mutatedsource is what vitest loads. (The console's dependency closure was built before the
type-check — see below — but that is a different requirement.)
Leg A — the fix removed (
maxLength: def.maxLength,— the pre-fix state)Anchors: fixed
1 → 0, mutant0 → 1, bytes64351 → 64329.Direction observed: turns red, as predicted, and the two DOM legs reproduce the card's
symptom literally — the object's ceiling arriving on the element the user types into.
Leg B — the object fallback dropped (
maxLength: override.maxLength,)This is what makes the fallback cases controls rather than decoration: they are scoped to
the same merge branch and they can fail.
Anchors: fixed
1 → 0, mutant0 → 1, bytes64351 → 64334.All five leg-A cases stay green here. Without these controls, "the form's 40 wins" would
be equally satisfied by a merge that lost the object side entirely.
Leg C — a fabricated floor (
override.maxLength ?? def.maxLength ?? 255)The remaining control asserts the attribute is absent when neither side declares a
ceiling; leg C is what proves that assertion can fail.
Anchors: fixed
1 → 0, mutant0 → 1, bytes64351 → 64358.Without it, every assertion above would be equally satisfied by a renderer that caps every
input at a number of its own choosing.
After all three legs the trap restored the tree to byte-identical with the commit
(
git diff HEADempty; fixed-anchor count back to1).Tests
Two homes, on purpose:
FormPage.test.ts— four cases in the existingbuildSectionsblock, next to thesibling
lets FormField overrides win over object defaultstest that named this contractand did not cover this key: override wins, object fallback still reaches the row, an
explicit
0survives (??-not-||), and the both-absent floor.FormPage.maxLength.test.tsx(new, 152 lines) — the end-to-end leg, against therendered attribute. A merge-level pin is necessary but not sufficient here: in this repo
that last step has independently failed for this key twice already (内建 form 分支的 input 字段:legacy
max_length拼法拿不到任何 maxlength 上限,只在 DOM 上留下一个失效属性 #5201, 内建 form 的default回退分支有和 #5201 完全相同的 ceiling 缺陷:max_length拿不到上限,只在 DOM 上留下失效属性 #5253, bothclosed, both "a declared
max_lengthceiling never reaches the DOM"), in the built-inform branches, by a different mechanism each time — which is exactly what a merge-level
pin cannot see. Both DOM sites are covered separately, because they are separate JSX
elements in separate switch branches.
Gates run locally, at
f078f7a9c— the final commitEvery result below was produced against this sha; nothing changed in the tree afterwards
(
git statusclean,git diff HEADempty).vitest run apps/console/(repo root)Test Files 67 passed (67)·Tests 730 passed (730)pnpm --filter @object-ui/console type-check0errors (tsc --noEmit && tsc -b tsconfig.node.json --force)check-changeset-presence.mjs✅ 4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/formpage-maxlength-override-5595.md.check-changeset-no-major.mjs✅ No changeset declares amajorbump.check-changeset-fixed.mjs✅ All workspace packages are in the changeset fixed group.check-control-bytes.mjs✅ check-control-bytes: OK (scanned 4681 tracked text file(s); skipped 85 binary).check-lint-coverage.mjs✅ lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).check-type-check-coverage.mjs✅ type-check coverage: 45/46 viatype-check… 1 not compiled.+✅ test type-check coverage: 41/41 packages compile their testscheck-phantom-dependencies.mjs✅ Every in-scope import is declared by the package that publishes it.check-package-self-import.mjs✅ No package names itself inside its own src/.Exit codes were captured before any pipe (
cmd > file 2>&1; EXIT=$?), and each rowquotes the gate's own verdict line rather than a bare
$?.Two toolchain notes worth recording:
type-checkreturned 340 errors, all cascading fromTS2307/TS2882against workspace packages — the unbuilt-dependency-closure trap in a fresh worktree, not
this change.
pnpm --workspace-concurrency=2 --filter '@object-ui/console^...' buildfirst, then
0errors. Judging the first run would have sent someone chasing anon-existent problem in
FormPage.tsx.--project '|@object-ui/console|'(copying the|…|decoration vitest prints around project names). It failed loudly —No projects matched the filter— rather than passing while running nothing.Lint — a declared narrowing, with its three pieces of evidence
pnpm lint(turbo run lint, 46 packages) is CI's run, and it was measured taking >5minutes in this container while another agent held the shared verify lock. What was run
instead is the complete lint population of the one package this PR touches —
eslint .insideapps/console, which is that package's ownlintscript verbatim:itself; no path list was supplied to it.
--format json— 167 files,errorCount0,warningCount201. The new
FormPage.maxLength.test.tsxappears in that enumeration with 0/0, so itis covered rather than silently ignored. The 201 warnings are pre-existing and none fall
in the edited region (
FormPage.tsxline 425–432); CI sets no--max-warnings, soerrors are the gated number.
eslint.config.jssets onlyecmaVersionandglobalsunderlanguageOptions; there is noparserOptions.projectand noprojectService, so typescript-eslint runs without type information. Each file isjudged from its own syntax in isolation, so a diff confined to
apps/consolecannot movea verdict on any file in the other 45 packages.
Broken gauges, noted and not fixed, per dispatch:
check-eager-closure-budgetexits 2 untila console build writes its report, and
check-doc-snippet-typesexits 1 until the workspaceis built.
Scope
out of scope: #5594— that card is already landed (PR #5624) and this branch only rebasesonto its result. No other defect was folded in; nothing outside
buildSections' merge loop,its two test homes and the changeset is touched.
Generated by Claude Code