fix(cli): guard convention/label-case on a localized label - #16280
Conversation
`checkLabelCase` indexed its argument (`label[0].toUpperCase()`) on a parameter annotated `string`, while every call site reaches it through `any`-typed config walking and `I18nLabelSchema` is `z.union([z.string(), InlineLocaleMapSchema])`. On the map form `label[0]` is `undefined`, so the rule threw a `TypeError` that escaped `lintConfig` into the command's catch-all: every `os lint` face exited 1 with `Cannot read properties of undefined (reading 'toUpperCase')`, naming no rule, no path and no remedy, on input `ObjectStackDefinitionSchema` parses clean. The rule now returns early unless `typeof label === 'string'`. The string branch is byte-identical, pinned per carrier. It deliberately says nothing about a localized label rather than resolving the map: picking which locale entry a case verdict is taken against is a product call, not a lint call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…nt-label-case-localized
The four rows in "the localized fixtures are schema-VALID" called
`normalizeStackInput(stack).stack`, but `normalizeStackInput` returns the
normalized stack itself, not a `{ stack }` wrapper — so `.stack` was
`undefined` and every row was parsing `undefined`, not the fixture. All four
were red.
Worse, the CONTROL row was passing for the wrong reason: it asserts a number
label does NOT parse, and `undefined` does not parse either, so it went green
without ever discriminating on the label. The block that exists to prove the
localized fixtures are supported authoring input was measuring nothing.
Drop the `.stack`. The four positives now parse clean and the control still
rejects, so the control discriminates on the label for the first time.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 349c8bd764ade8bba6f873bcf6ae1955459d143d && git checkout 349c8bd764ade8bba6f873bcf6ae1955459d143d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2756e07d10f21170e6121ee9125d3c83547ef1fc 764c049adb81db5d44ac646890a09d21b4e6301f && git checkout -B drift-repro 2756e07d10f21170e6121ee9125d3c83547ef1fc && git merge --no-ff 764c049adb81db5d44ac646890a09d21b4e6301f
node scripts/docs-audit/affected-docs.mjs --json 2756e07d10f21170e6121ee9125d3c83547ef1fc
|
`Check Changeset`'s level axis refuses a PR that declares clause ② and
grades a package it grew `patch`. This PR declares clause ② — the
`needs:contract-review` carrier is on it — and the conformance limb fires:
`convention/label-case`'s JSON face moves from `{"error": ...}` / exit 1 to
`{"passed": true, ...}` / exit 0 on an input class
`ObjectStackDefinitionSchema` parses clean. `minor` is the grade that
declaration implies — a purely additive widening of a published package's
public surface takes at least `minor` (maintainer ruling, 2026-09-04
decision batch #35), and the commit type may raise a bump but never lower
it. The two declarations now agree inside one PR.
Also replaces a falsification condition in the NO MOVE header that did not
falsify. It offered `a label != null guard that changes the empty-string
case` as a third way to swallow strings; it is neither. On a lowercase
string `label != null` is true, so the row still reports and NO MOVE stays
green; on `''` both spellings reach the falsy `label &&` test and return
null, so the empty-string case does not move either. Replaced with the
guard inverted to `typeof label === 'string'`, which does make every
lowercase row below stop reporting — the observable the header claims.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Landing provenance —
|
Fixes #15880
os lintcrashed with a bareTypeError: Cannot read properties of undefined (reading 'toUpperCase')on a config thatObjectStackDefinitionSchemaparses clean.checkLabelCaseinpackages/cli/src/commands/lint.tsindexed its argument (label[0].toUpperCase()) on a parameter annotatedstring, while every call site reaches it throughany-typed config walking.I18nLabelSchemaisz.union([z.string(), InlineLocaleMapSchema]), so on the inline locale map formlabel[0]isundefined. The throw escapedlintConfiginto the command's catch-all: an author who localized an app label or a list-view label could not lint the project at all, on either face, and the message named no rule, no path and no remedy.The rule now returns early unless
typeof label === 'string'. That is the whole change.Scope: the guard, deliberately not a locale-aware case check
Per triage, the deliverable is that
os lintstops crashing — not a decision about how a localized label should be case-checked. Resolving the map and case-checking one entry would decide which locale entry is authoritative for a lint verdict, which is a product call this PR does not make. So the rule says nothing about a localized label, and the tests assert the absence of aconvention/label-caseissue, so that widening the rule later has to rewrite those assertions on purpose rather than satisfy them silently.How many call sites reach the rule
lintConfigwalks five collections —objects,views,apps,flows,agents— and callscheckLabelCasefrom 4 sites, reaching 5 authoring paths (getViewLabelresolves to either thelistor thelistViews.*path):lint.ts:202objects[].labelobject.zod.ts:1621—z.string().optional()lint.ts:229objects[].fields.*.labelfield.zod.ts:933—z.string().optional()lint.ts:254views[].list.label/views[].listViews.*.labelview.zod.ts:1819—I18nLabelSchema.optional()lint.ts:268apps[].labelapp.zod.ts:1291—I18nLabelSchemaThe sweep filed on the card found exactly two hits because its fixture reached exactly two. The class is the set of call sites, not the set of sweep hits — which is why
views[].listViews.*.labelis pinned here even though no sweep reached it.Reachability observation (measured, not fixed here)
I18nLabelSchemais imported by nineui/schemas —app,view,page,dashboard,chart,report,action,bulk-action,component— butlintConfignever walkspages,dashboards,charts,reports,actions,bulkActionsorcomponents. Soconvention/label-casenever reaches those labels at all, before or after this change. The rule's coverage is narrower than the schema surface it nominally governs. Reported for triage, not touched here.Both faces, before and after
Fixture: an app with
label: { en: 'Todos', 'zh-CN': ... }plus a minimal manifest, driven through the built CLI.Before (main's
lint.tsrestored over the fix, CLI rebuilt):After:
What it now says, and that it is true: the linter finishes and reports exactly one warning —
protocol/missing-engines-rangeatmanifest.engines.protocol, which is a real property of the fixture and unrelated to the label — with0 errors,0 schemaand grade A. It emits noconvention/label-caseissue for the localized label, and it does not report that label as missing either. A guard that merely silenced the crash while emitting a spuriousrequired/labelis pinned against explicitly.Behaviour on a plain string label does not move
Pinned per carrier rather than asserted: all five carriers keep the same warning, message,
fixvalue and path. The ablation below is what proves it — under the mutation those rows stay green, i.e. this branch andmainagree on every string carrier.Ablation
Falsification conditions named and directions predicted before running. Mutation =
main'slint.tsrestored over the fix; proven on disk by blob hash both ways, restored under anEXIT INT TERMtrap with absolute paths, restore proven by observed state.os lint --scoreTypeErroros lint --score --json{"error": ..., "conversions": []}lint.tslintErrorsetscore-lint-crash.test.tsMutation proof: source blob
d6c14bb15f16...(main) with the guard marker absent, and the marker confirmed absent indist/so the CLI ran mutated code. Restore proof: blob back to611e6613c33b...,git diff HEADempty, marker present in source and indist/again — observed state, not an exit code.A repair to the pins themselves
The schema-valid block called
normalizeStackInput(stack).stack, butnormalizeStackInputreturns the normalized stack directly, not a wrapper..stackwasundefined, so all four rows were parsingundefinedand were red. Its CONTROL row — "a number label does NOT parse, so the parse check discriminates" — was passing for the wrong reason, sinceundefineddoes not parse either. The block that exists to prove the localized fixtures are supported authoring input was measuring nothing. Dropping.stackmakes the four positives parse clean and leaves the control rejecting, so the control discriminates on the label for the first time.For triage: a schema asymmetry this PR deliberately did not touch
Measured here, not changed, per the binding instruction not to unify the two declarations in this PR:
objects[].label(packages/spec/src/data/object.zod.ts:1621) isz.string().optional()objects[].fields.*.label(packages/spec/src/data/field.zod.ts:933,FieldSchema) isz.string().optional()apps[].label(packages/spec/src/ui/app.zod.ts:1291,AppSchema) isI18nLabelSchemaviews[].list.label(packages/spec/src/ui/view.zod.ts:1819) isI18nLabelSchema.optional()So an app label may be localized and an object label may not. Narrowing or widening either is a published-declaration change and needs a human floor. Flagged for the dispatching seat to file; not filed from here.
Two anchors above were corrected after review. They were inherited from triage and were pointing at the wrong member; both were re-measured against the tree at this PR's merge base and the citations now name the schema each line actually belongs to.
app.zod.ts:300isAppSchema.labelBaseNavItemSchema.label(BaseNavItemSchemaopens atapp.zod.ts:295)app.zod.ts:1291field.zod.ts:299is the field base label,z.string()SelectOptionSchema.label(SelectOptionSchemaopens atfield.zod.ts:288)field.zod.ts:933, and it isz.string().optional(), notz.string()Both replacements are still
I18nLabelSchemaand still a plain string respectively, so every conclusion drawn above is unchanged — the accepts-the-map column, the 4-call-site/5-authoring-path count and the asymmetry itself were all driven independently of these line numbers. The one substantive correction is.optional(): with it, both object-side labels arez.string().optional(), so there is no required-vs-optional asymmetry between them. The only asymmetry is plain string vsI18nLabelSchema, which is what #16282 records. The other two anchors (object.zod.ts:1621,view.zod.ts:1819) were re-measured too and are correct as written.Verification
At head
c8d31a3fe0e— the implementationpnpm --filter '@objectstack/cli^...' build—VERDICT command-exit 0pnpm --filter @objectstack/cli exec vitest runover 7 lint/score files —VERDICT command-exit 0, 7 files / 68 tests passedpnpm --filter @objectstack/cli typecheck—VERDICT command-exit 0;check:test-typecheck: OK. All three edited files confirmed present in thetsconfig.test.jsonprogram via--listFiles, and zero errors in them (the ledger's 3 files / 28 errors are pre-existing and untouched)node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack—Reconciliation — 57 famil(ies). Targeted subset run locally, all exit 0:check-changeset-no-major,check-empty-changeset,check:nul-bytes,check:test-source-alias,check-changeset-fixed,check:objectui-changeset,check:changeset-gate-self-tests. The farm is CI's to run in full.At head
764c049adb8— the changeset levelThe first head graded
@objectstack/clipatchwhile declaring clause ②, andCheck Changesetrefused it. The level is nowminor.That axis is only readable from a
pull_requestpayload, so it is driven here with--eventcarrying this PR's live label set (documentation,size/m,tests,tooling,needs:contract-review) and its body. Same payload and same--base origin/mainin both runs — only the tree differs, by the one word:The control is what makes the green informative: it reproduces the exact CI refusal from the same payload, so the pass is a read axis rather than an unread one.
node scripts/check-changeset-no-major.mjs --base origin/mainform printsLEVEL AXIS: NOT MEASUREDand exits 0 on both trees. It cannot distinguish them, and reading that exit code as a pass is what let the wrong level reach CI. Drive it with--event.A falsification condition in the test header was also replaced, because it did not falsify. The NO MOVE property offered
a label != null guard that changes the empty-string caseas a third way to swallow strings. It is neither: on a lowercase stringlabel != nullis true, so the row still reports; on''both spellings reach the falsylabel &&test and return null. Measured as a pair, mutatinglint.tson disk each way (blob hash before/after, restored under anEXIT INT TERMtrap, restore proven bygit diff HEADempty):if (label == null) return null;— as the header claimedif (typeof label === 'string') return null;— the replacementThe header now names the inverted guard, which produces the observable it claims: every lowercase row stops reporting.
Also at this head, all exit 0:
check:nul-bytes,check:changeset-gate-self-tests,check:empty-changeset,check:objectui-changeset,check:changeset-fixed,check:doc-authoring,check-comment-mask-adoption,check-adr-0087-registration --base origin/main, plus the@objectstack/clidependency-closure build and the package's own build,@objectstack/clitypecheck (check:test-typecheck: OK, the 3-file / 28-error debt ledger unchanged), and the full@objectstack/clivitest suite — all 270 test files green at this head, measured in two passes: 263 files in the first (3194 passed | 6 expected fail | 31 skipped), and the remaining 7 in a second pass (7 passed, 31 tests). Those 7 drive the PUBLISHED entry and needpackages/cli/dist, which the first pass lacked because it had built only the dependency closure (@objectstack/cli^..., which excludes the package itself); they reportedpackages/cli is not built— a missing prerequisite, not a finding — and pass once the package's own build exists.Changeset:
minoron@objectstack/cli. A bug fix by shape, but the PR declares clause ② and the conformance limb fires — the JSON face moves from{"error": ...}/ exit 1 to{"passed": true, ...}/ exit 0 on an input classObjectStackDefinitionSchemaparses clean — and a declared widening of a published surface takes at leastminor(maintainer ruling, 2026-09-04 decision batch #35).Generated by Claude Code