Uh oh!
There was an error while loading. Please reload this page.
fix(cli): give a root-level validation issue a Path line - #7038
Merged
Conversation
`objectui validate` guarded its Path line with `issue.path.length > 0`, so an issue at `path: []` printed no Path line at all — silent in exactly the case a reader most needs oriented. That case is the common one, not an edge. `safeValidateSchema` runs `AnyComponentSchema`, a `z.union` over every component arm, so ANY document matching no arm reports one top-level issue at the root: `invalid_union` · `Invalid input` · `path: []`. Measured on the parent commit, a menu carrying the divider spelling retired in objectui#6523 printed a bare verdict on a whole document, with nothing saying which node had been judged. Every reported issue now carries a Path line; a root-level one reads `Path: (root)`, parenthesised so it cannot be read as a real key named `root`. Non-root issues print their authored path unchanged — pinned by a control case, since a fix printing `(root)` unconditionally would satisfy a root-only test while destroying the paths authors depend on. Scope: only top-level issues are read, as before. Whether a failing union should also surface its per-arm diagnoses — and if so which arm's — is an author-facing diagnostic contract left open on objectui#7004 for a maintainer ruling; `issue.errors` is deliberately not walked, and a case pins that boundary so the ruling lands as a deliberate edit rather than a silent widening.
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-sam
commented
Aug 31, 2026
CollaboratorAuthor
Attribution note: this repo's body sanitizer strips the trailing attribution footer on every PR-body edit — measured three times on this PR (present on create, gone after each PATCH). It is recorded here instead of re-pasted into the body, and the durable session reference is in the body's opening prose. Generated by Claude Code |
os-sam
marked this pull request as ready for review
August 31, 2026 15:17
Uh oh!
There was an error while loading. Please reload this page.
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.
Part of #7004 — the mechanical half only.
Seat session:
https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yBWhich half this is
#7004 was split in triage into a mechanically decidable half and a maintainer ruling. This PR does the first and deliberately leaves the second untouched:
Pathline whenever the issue path was empty. Printing something there decides nothing about union semantics.issue.errorsis not walked, and finding(cli):validate/checkprint only TOP-LEVEL zod issues, so every union-shaped schema reports a bare "Invalid input" and the per-arm guidance is never seen #7004 stays open after this merges (hencePart of, not a closing keyword).The defect
packages/cli/src/commands/validate.tsguarded its Path line withissue.path.length > 0, so an issue at the document root printed no Path line at all — silent in exactly the case a reader most needs oriented.That case is the common one, not an edge.
safeValidateSchemarunsAnyComponentSchema, which is a union over every component arm, so any document matching no arm reports one top-level issue at the root:invalid_union·Invalid input· empty path.Reproduced before the fix, not after
Measured on the unmodified base commit
350509b53, authoring a menu that carries the divider spelling retired in #6523:{ "type": "dropdown-menu", "items": [{ "label": "New Tab", "type": "separator" }] }Before — a bare verdict on a whole document, nothing saying which node was judged:
After:
(root)is parenthesised so it cannot be read as a real key literally namedroot— a genuine path to one prints asroot.The reverse verification is a real run, not only prose: with
validate.tsreverted to the base blob and the mutation confirmed on disk by blob hash, the three root-oriented cases fail (expected ... to contain 'Path: (root)', andissue "1. Invalid input" printed no Path line) while the three control cases stay green. The tree was restored afterwards and the restore proven by blob-hash comparison against HEAD.The non-root case is pinned too
A fix that printed
(root)unconditionally would pass a root-only test while destroying every real path. So the suite asserts the other side of the guard:{ "type": "form", "fields": [{ "name": "pw", "widget": "ui:password" }] }still printsPath: fields → 0 → widgetand the output contains no(root)anywhere. A valid document still prints no Path line and exits 0.One case is structural rather than by-example — every numbered issue must be followed by a Path line — because the defect was an absence, and one example line would not catch a future guard reopening the hole on some other issue shape.
Two premise corrections from verification
1.
check.tsdoes not have this defect — it has no zod-issue printer at all. The card and the dispatch both say it "prints the same three fields the same way". It does not:check.tscallssafeValidateSchema(content).successpurely as a boolean recogniser (the marker gate from #5127 / #6075) and never reads.issues.git log -Sconfirms aPath:printer has never existed in that file.packages/cli/src/commands/validate.tsis the only zod-issue printer in the CLI.I deliberately did not make
check.tsstart printing issues. ItssafeValidateSchemacall answers "is this file ours?", and printing the issues behind a negative recognition would flood the report with diagnoses of files that simply are not ObjectUI documents — the precise failure #5127 and #6075 exist to prevent.2. The repro recipe in the card does not reach the CLI as written.
{ label: 'New Tab', type: 'separator' }at the document rootvalidates and exits 0 — the CLI validates the root againstAnyComponentSchema, not againstMenuItemSchema. The card's measurement was taken againstMenuItemSchemadirectly, where the shape it describes is exactly right. To exercise the same defect through the CLI the item has to sit in a menu, which is what the fixture above does.Shared renderer: deliberately not created, and why
The dispatch asked me to decide whether the two printers should share one renderer rather than each carrying a copy. Following premise correction 1: there is only one printer. There is no duplication to converge, so there is nothing here matching the "one contract, several hand-written copies" pattern closed in #5040 / #5596 / #6247 / #6887 / #7014 — extracting a renderer for a single call site would be speculative generality, and the only plausible second consumer (
check.ts) must not print issues at all.If the arm-selection ruling later adds a second issue-rendering surface, that is the moment a shared renderer earns its keep — with a real second caller to shape it.
The scope boundary is pinned, not just asserted
One case records that a union prints exactly one top-level entry and none of the per-arm #6523 remediation text. It pins a boundary, not a desired end state: when the arm-selection ruling lands it is expected to change with it, so that widening arrives as a deliberate edit rather than silently.
Checks
All run at
8641b8cd, the final commit.pnpm exec vitest run packages/cli/Test Files 15 passed (15)·Tests 236 passed (236)validate.tsreverted to base blobpnpm --filter @object-ui/cli run type-check--listFilesconfirms both changed files are in the program (1 hit each)pnpm --filter @object-ui/cli run lint11 problems (0 errors, 11 warnings)— all pre-existing, none in the changed filesnode scripts/check-changeset-presence.mjs1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjsNo changeset declares a major bumppnpm check:control-bytesOK (scanned 5844 tracked text file(s))pnpm check:shell-escape-residueOK (4/4 roots resolved)pnpm check:vi-mock-specifiers,check:vi-mock-inheritOKpnpm lint:coverage46/46 packages linted, 0 with outstanding errorspnpm check:doc-fencesOKpnpm check:phantom-deps,check:self-importOKeslint here is narrowed to the affected package rather than the whole repo, and that narrowing is a measurement: this repo's root
lintisturbo run lint, i.e. each package runningeslint src, so the package run is the job CI runs for these files; andeslint.config.jsconfigures no type-aware linting (noprojectService, noproject:), so this diff cannot move the verdict on any untouched file.Docs need no update:
content/docs/utilities/cli.mdxdocuments the command but shows no sample failure output, so no published example goes stale.